Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Properly handle object properties on Query Compiler #413

Merged

Conversation

csansoon
Copy link
Contributor

@csansoon csansoon commented May 14, 2024

Describe your changes

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

  • Invoke object properties as methods
  • Modify object properties values
  • Allow optional chaining operator (?.)

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

  • I have added thorough tests
  • I have updated the documentation if necessary
  • I have added a human-readable description of the changes for the release notes
  • I have included a recorded video capture of the feature manually tested

Copy link

changeset-bot bot commented May 14, 2024

🦋 Changeset detected

Latest commit: d33e41c

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 csansoon changed the base branch from main to canary May 14, 2024 08:35
@csansoon
Copy link
Contributor Author

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?

@csansoon csansoon force-pushed the fix/compiler-can-run-methods-from-scoped-variables-canary branch 2 times, most recently from b98a332 to b2fee7f Compare May 16, 2024 10:51
@geclos
Copy link
Contributor

geclos commented May 21, 2024

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

expect(result).toBe('$[[val]]')
})

it('allows optional chaining operator', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we allow it here but not here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because you can't do undefined = 3. That's something internal in the compiler, can't even change it. Anyways, that does not work in regular javascript either.

@csansoon csansoon force-pushed the fix/compiler-can-run-methods-from-scoped-variables-canary branch from b2fee7f to d33e41c Compare May 22, 2024 12:00
@csansoon csansoon merged commit 3e87858 into canary May 22, 2024
4 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators May 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants