Skip to content

Conversation

mariusandra
Copy link
Collaborator

@mariusandra mariusandra commented Jun 8, 2023

Problem

You can't access array elements in properties

Changes

Makes the following cases work:

  • properties.string
  • properties.array_str.1
  • properties.array_str[1]
  • properties.obj_array.id.1
  • properties.obj_array.id[1]
  • properties.array_array_str.1.1
  • properties.array_array_str[1][1]
  • properties.array_obj_array.1.id.1
  • properties.array_obj_array[1]['id'][1]
  • properties.array_obj_array_obj.1.id.1.id
  • properties.array_obj_array_obj[1].id[1].id
  • properties.array_obj_array_obj[1]['id'][1]['id']
  • properties.array_obj.1.id
  • properties.array_obj[1].id

Sadly for all of us, SQL indexes start from 1. This is bound to cause confusion, so I added a helpful error if you use 0.

SQL indexes can also be negative, so we can't automatically +1 in the HogQL layer either 🙃

How did you test this code?

Plenty of test changes. Tried it in the UI as well.

Out of scope

We could fork ClickHouse

@samuelstroschein
Copy link

@PostHog I love subscribing to issues and know that a feature we need is going to be implemented <3

@jannesblobel subscribe to this issue. When this issue is complete we can build insights to derive what plugins are used.

@mariusandra
Copy link
Collaborator Author

Haha, great to hear! There are a few more complex cases I still need to get working before it's ready. Unlikely this will be done, reviewed and merged today, but soon 👀

@mariusandra mariusandra marked this pull request as ready for review June 13, 2023 08:20
Copy link
Contributor

@thmsobrmlr thmsobrmlr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mariusandra mariusandra merged commit 73006bd into master Jun 13, 2023
@mariusandra mariusandra deleted the json-arrays branch June 13, 2023 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants