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

Disable query AST population for now #1298

Merged
merged 4 commits into from
Jan 30, 2025
Merged

Disable query AST population for now #1298

merged 4 commits into from
Jan 30, 2025
+21 −16

Conversation

shangyian
Copy link
Contributor

@shangyian shangyian commented Jan 30, 2025 •
edited
Loading

Summary

This change disables the query AST population for now, from #1280. I didn't fully undo that PR because some changes from there are still valuable. The primary reasons for this are:

  • There are sometimes query building discrepancies
  • We need to audit our database calls to defer pulling the query_ast in most cases, because it is a large object

Test Plan

  • PR has an associated issue: #
  • make check passes
  • make test shows 100% unit test coverage

Deployment Plan

@shangyian shangyian requested a review from agorajek January 30, 2025 03:01
@netlify Netlify
Copy link

netlify bot commented Jan 30, 2025 •
edited
Loading

✅ Deploy Preview for thriving-cassata-78ae72 canceled.

Name Link
🔨 Latest commit 8e58b91
🔍 Latest deploy log https://app.netlify.com/sites/thriving-cassata-78ae72/deploys/679aeb9d3e49480008b6be18

@netlify Netlify
Copy link

netlify bot commented Jan 30, 2025 •
edited
Loading

✅ Deploy Preview for thriving-cassata-78ae72 canceled.

Name Link
🔨 Latest commit 3bfb12b
🔍 Latest deploy log https://app.netlify.com/sites/thriving-cassata-78ae72/deploys/679b03e00f240d0008657d59

@shangyian shangyian marked this pull request as ready for review January 30, 2025 03:15
@shangyian shangyian added the bug Something isn't working label Jan 30, 2025
@shangyian shangyian merged commit 0c922c1 into DataJunction:main Jan 30, 2025
16 checks passed
@shangyian shangyian deleted the disable-query-ast branch January 30, 2025 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agorajek agorajek Awaiting requested review from agorajek

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@shangyian