latitude-dev / latitude

Developer-first embedded analytics
https://latitude.so
GNU Lesser General Public License v3.0
716 stars 30 forks source link

Fix: Properly handle object properties on Query Compiler #413

Closed csansoon closed 3 weeks ago

csansoon commented 1 month ago

Describe your changes

Compiler did not properly handle object properties. Before this commit, you could not:

Invoke object properties as methods

Some variable types can contain functions that may be useful in the query compilation run-time. A clear example is Dates, where a Date can be passed to a query as a param, and we may want to run date-related methods.

{minDate = param('start_date')}  -- Returns a Date object

SELECT *
FROM table
WHERE year >= {minDate.getYear()}   -- You can now run the .getYear() method from the date object!

Modify object properties as methods

You can now also modify the value from object properties such as arrays and hashes.

{ results = runQuery('other_query') }
{ results[0].name = 'foo' }  -- Modifying a property from an object

SELECT *
FROM table
WHERE name IN {#each results as row} {row.name} {/each}

Allow optional chaining operator (?.)

The optional chaining operator allows you to read the value of a property located deep within a chain of connected objects without having to expressly validate that each reference in the chain is valid.

{ results = runQuery('other_query') }

SELECT *
FROM table
WHERE name = {results[0]?.name ?? 'default'}  -- If there are no results, it will return 'default'

Checklist before requesting a review

changeset-bot[bot] commented 1 month ago

🦋 Changeset detected

Latest commit: d33e41c54202a0ae50e493a184e5617fe94e2e3b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages | Name | Type | | ------------------------------------- | ----- | | @latitude-data/sql-compiler | Minor | | @latitude-data/server | Patch | | @latitude-data/source-manager | Patch | | @latitude-data/cli | Patch | | @latitude-data/athena-connector | Patch | | @latitude-data/bigquery-connector | Patch | | @latitude-data/clickhouse-connector | Patch | | @latitude-data/databricks-connector | Patch | | @latitude-data/duckdb-connector | Patch | | @latitude-data/materialized-connector | Patch | | @latitude-data/mssql-connector | Patch | | @latitude-data/mysql-connector | Patch | | @latitude-data/postgresql-connector | Patch | | @latitude-data/snowflake-connector | Patch | | @latitude-data/sqlite-connector | Patch | | @latitude-data/test-connector | Patch | | @latitude-data/trino-connector | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

csansoon commented 1 month ago

Since now we can do some things like {results[0]?.name ?? 'default'}, I was thinking we could also do something like {param('start_date')?.getYear()}. This would mean changing the param function where, if the parameter does not exist, it should return undefined instead of raising an error. What do you think? Would it be better to crash, or just return undefined?

In the case of SELECT * FROM user WHERE id = {param('user_id')}, if no user is given that would not fail, but just return an unexpected response. I don't know what's better for the user in this case. Thoughts?

geclos commented 3 weeks ago

I agree on supporting object properties on param() calls but I don't understand why it means we need to allow param returning undefined.