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.

Sign up for GitHub

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

Jump to bottom

BREAKING CHANGE: Relative queries #397

Closed
wants to merge 1 commit into from
Closed

BREAKING CHANGE: Relative queries #397

wants to merge 1 commit into from

Conversation

andresgutgon
Copy link
Contributor

@andresgutgon andresgutgon commented May 10, 2024 •
edited
Loading

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

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 changeset-bot
Copy link

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

🦋 Changeset detected

Latest commit: 3fe0fa9

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

andresgutgon
andresgutgon commented May 10, 2024
View reviewed changes
apps/server/scripts/run_query/index.ts
@@ -40,12 +39,9 @@ async function runQuery(
debug = false,
) {
try {
const { source, sourceFilePath } = await sourceManager.loadFromQuery(query)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a bug in my previous PR I forgot to test this

@andresgutgon andresgutgon force-pushed the feature/get-sql-from-query-file-in-source-and-pass-to-connector branch from 12ccc58 to caba6d1 Compare May 10, 2024 13:59
andresgutgon
andresgutgon commented May 10, 2024
View reviewed changes
packages/source_manager/src/baseConnector/supportedMethods.test.ts Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@csansoon please review this file 👇 Good stuff

@andresgutgon andresgutgon force-pushed the feature/get-sql-from-query-file-in-source-and-pass-to-connector branch 2 times, most recently from 936a097 to 187b29e Compare May 10, 2024 15:11
@andresgutgon andresgutgon changed the title Refactor compile query to pass SQL string from query file to the connector BREAKING CHANGE: Relative queries May 10, 2024
@andresgutgon andresgutgon force-pushed the feature/get-sql-from-query-file-in-source-and-pass-to-connector branch from 187b29e to 61d36a8 Compare May 10, 2024 15:19
@andresgutgon andresgutgon force-pushed the feature/get-sql-from-query-file-in-source-and-pass-to-connector branch from 61d36a8 to cf6ab16 Compare May 10, 2024 15:26
@andresgutgon andresgutgon removed the WIP label May 10, 2024
@andresgutgon andresgutgon mentioned this pull request May 10, 2024
Closed
2 tasks
andresgutgon
andresgutgon commented May 10, 2024
View reviewed changes
apps/server/src/lib/query_service/find_or_compute.test.ts
source,
connectionParams: { fail: false },
})
source.setConnector(connector)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No more setConnector 🔥

andresgutgon
andresgutgon commented May 10, 2024
View reviewed changes
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']) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead source['_connector']

@andresgutgon andresgutgon force-pushed the feature/get-sql-from-query-file-in-source-and-pass-to-connector branch from cf6ab16 to 433ce21 Compare May 10, 2024 15:54
@andresgutgon andresgutgon force-pushed the feature/get-sql-from-query-file-in-source-and-pass-to-connector branch 3 times, most recently from 9dd7b48 to a95ceae Compare May 13, 2024 09:19
@andresgutgon
Refactor compile query to pass SQL string from query file
3fe0fa9 
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 andresgutgon force-pushed the feature/get-sql-from-query-file-in-source-and-pass-to-connector branch from a95ceae to 3fe0fa9 Compare May 13, 2024 10:18
@andresgutgon
Copy link
Contributor Author

Re-open here #416

@github-actions github-actions bot locked and limited conversation to collaborators May 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@csansoon csansoon csansoon approved these changes

Assignees
No one assigned
Labels
TRENECITO 🚂
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@andresgutgon @csansoon