BREAKING CHANGE: Relative queries #416

andresgutgon · 2024-05-14T09:11:33Z

Describe your changes

Until now we were requiring users to specify the queries in the ref function from the root of the source folder. Example:

MY_QUERIES
  - my-source
    - source.yml
    - query.sql
    - another_query.sql

Given that folder structure in query.sql

Before 🚫

-- MY_QUERIES/my-source/query.sql
SELECT * FROM {ref('my-source/another_query')}

Now ✅

-- MY_QUERIES/my-source/query.sql
SELECT * FROM {ref('./another_query')}

Or ✅

-- MY_QUERIES/my-source/query.sql
SELECT * FROM {ref('another_query')}

Or ✅

-- MY_QUERIES/my-source/query.sql
SELECT * FROM {ref('/my-source/another_query')}

Issue ticket number and link

#369

Dependency

https://github.com/latitude-dev/latitude/pull/414/files

TODO

Pass SQL content from query file from source to connector
Include in the context request that SQL text
Fix runQuery 🐛. I introduced the bug here
Use in ref method the SQL
Fix tests
QA is working the changes
Do Changeset

changeset-bot · 2024-05-14T09:11:37Z

🦋 Changeset detected

Latest commit: 8a6b394

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

This PR includes changesets to release 16 packages

Name	Type
@latitude-data/source-manager	Major
@latitude-data/server	Major
@latitude-data/sql-compiler	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/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

geclos · 2024-05-14T09:51:40Z

apps/server/src/lib/query_service/find_or_compute.test.ts

+async function buildConnector(queryPath: string) {
+  const source = await sourceManager.loadFromQuery(queryPath)
+
+  if (!source['_connector']) {


what's this check about?

connector is lazy loaded when a source compiles a query. Until that moment connector is not initialized

This is done to avoid injecting source

I still don't get it sorry could you add a comment with explanations as to why we need this defense?

We want to have expectations on the underlying connector. But the connector is not accessible from the outside. On top of that _connector is null until you load the good query because it's lazily evaluated.

hmm ok, the comment is good enough for now 👌🏼

GPT4o did for me. Is so nice

apps/server/src/lib/query_service/find_or_compute.test.ts

geclos · 2024-05-14T09:52:44Z

apps/server/src/lib/query_service/find_or_compute.ts

  query,
  queryParams,
  force,
 }: {
-  source: Source


much better 👌🏼

geclos · 2024-05-14T09:53:56Z

docs/queries/advanced/reference-queries.mdx

+-- queries/users/orders/query.sql
+SELECT * FROM {ref('../query.sql')} -- This will reference queries/users/query.sql
+```
+


review the rest of this documentation because i think there are some things wrong now

also we should explain the difference between /query_path query_path and ./query_path

Review the docs and changed docs about queries path resolution

geclos · 2024-05-14T09:56:21Z

packages/source_manager/src/baseConnector/connectorFactory.ts

@@ -96,6 +81,9 @@ export default async function createConnectorFactory({
  connectorOptions: ConnectorOptions<ConnectorAttributes>
 }): Promise<BaseConnector> {
  const packageName = getConnectorPackage(type)
-  const ConnectorClass = await importConnector(packageName)
+  const ConnectorClass = !packageName
+    ? TestConnector // If no package is found, use the test connector


a bit worried about all the code changes caused by the tests. Not blocking but I'd love if we could find ways to test code without having to adapt it to tests

The factory is super magic. It expects an npm package to exist with that name and to be installed in the source_manager. This is super annoying for testing. The way it is done now is that the test connector is a local file in this package.

packages/source_manager/src/baseConnector/index.ts

geclos · 2024-05-14T10:01:18Z

packages/source_manager/src/types.ts

+  request: CompileQueryRequest
+  // Parameters used in the query
+  accessedParams: QueryParams
+  // Parameters resolved by the connector, in order of appearance


I still get confused by the resolved name. What does it mean to resolve a parameter?

I think it means computing their value. cc @csansoon

geclos

Awesome work! This is super delicate though – We need to manually test query references in multiple scenarios:

/query
    source.yml
    query1.sql
    /folder1/query2.sql -- references query1
    /folder1/source2.yml
    /folder1/query3.sql -- uses source2.yml
    /folder1/folder2/query4.sql -- references query3

Running query1, query2, query3 and query4 should work ☝🏼

We want to move the responsability of fining the query in the file system from the connector to the source. After the last refactor now this is easier to todo because we have access to the source and the source manager in the connector. With this change is easier to pass the the full path of the query and look for relative paths and paths that lives outside the source of the connector. We want to do both things in the upcoming changes

andresgutgon added the TRENECITO 🚂 label May 14, 2024

This was referenced May 14, 2024

BREAKING CHANGE: Relative queries #397

Closed

Materialized connector inital setup #417

Merged

Base automatically changed from feature/make-source-available-to-connectors to canary May 14, 2024 09:34

andresgutgon force-pushed the feature/find-query-files-with-relative-paths branch from 439d804 to 23e5c64 Compare May 14, 2024 09:44

geclos reviewed May 14, 2024

View reviewed changes

apps/server/src/lib/query_service/find_or_compute.test.ts Outdated Show resolved Hide resolved

geclos reviewed May 14, 2024

View reviewed changes

apps/server/src/lib/query_service/find_or_compute.ts

query,

queryParams,

force,

}: {

source: Source

Copy link

Contributor

geclos May 14, 2024

Choose a reason for hiding this comment

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

much better 👌🏼

geclos reviewed May 14, 2024

View reviewed changes

packages/source_manager/src/baseConnector/index.ts Outdated Show resolved Hide resolved

geclos reviewed May 14, 2024

View reviewed changes

geclos approved these changes May 14, 2024

View reviewed changes

andresgutgon force-pushed the feature/find-query-files-with-relative-paths branch 2 times, most recently from f7a3f23 to 2d36079 Compare May 14, 2024 10:38

andresgutgon force-pushed the feature/find-query-files-with-relative-paths branch from 2d36079 to 8a6b394 Compare May 14, 2024 11:01

andresgutgon merged commit a8d4658 into canary May 14, 2024
4 checks passed

andresgutgon deleted the feature/find-query-files-with-relative-paths branch May 14, 2024 11:05

github-actions bot locked and limited conversation to collaborators May 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BREAKING CHANGE: Relative queries #416

BREAKING CHANGE: Relative queries #416

andresgutgon commented May 14, 2024

changeset-bot bot commented May 14, 2024 •

edited

Loading

geclos May 14, 2024

andresgutgon May 14, 2024

andresgutgon May 14, 2024

geclos May 14, 2024

andresgutgon May 14, 2024

andresgutgon May 14, 2024 •

edited

Loading

geclos May 14, 2024

andresgutgon May 14, 2024

geclos May 14, 2024

geclos May 14, 2024

geclos May 14, 2024

andresgutgon May 14, 2024 •

edited

Loading

geclos May 14, 2024

andresgutgon May 14, 2024

geclos May 14, 2024

andresgutgon May 14, 2024

geclos left a comment •

edited

Loading

BREAKING CHANGE: Relative queries #416

BREAKING CHANGE: Relative queries #416

Conversation

andresgutgon commented May 14, 2024

Describe your changes

Before 🚫

Now ✅

Issue ticket number and link

Dependency

TODO

changeset-bot bot commented May 14, 2024 • edited Loading

🦋 Changeset detected

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andresgutgon May 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andresgutgon May 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geclos left a comment • edited Loading

Choose a reason for hiding this comment

changeset-bot bot commented May 14, 2024 •

edited

Loading

andresgutgon May 14, 2024 •

edited

Loading

andresgutgon May 14, 2024 •

edited

Loading

geclos left a comment •

edited

Loading