Fix BigQuery quoting #858
Merged
Fix BigQuery quoting #858
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Not sure this will be the end of it, but progress
|
(I need to do some checks before merging, e.g. should the * be outside the backticks) |
|
(Continuing on from #852) It's actually a bit more complicated than this! Because it's required to have the column name (or SELECT
`bigquery-public-data.pypi.file_downloads`.country_code
-- or
-- `bigquery-public-data.pypi.file_downloads`.*
FROM
`bigquery-public-data.pypi.file_downloads`
WHERE
timestamp = '2021-08-01'This is not valid: SELECT
- `bigquery-public-data.pypi.file_downloads`.country_code
+ `bigquery-public-data.pypi.file_downloads.country_code`
\--or
--- `bigquery-public-data.pypi.file_downloads`.*
+-- `bigquery-public-data.pypi.file_downloads.*`But the rule of "leave the final ident outside of the quotes" doesn't work, because the whole table must be quoted in the So we probably need to branch higher in the translator and operate differently for column vs. table names. Another option would be to have more semantic understanding of the identifier — e.g. whether something is a table vs a column name — in the translator. Probably longer term this has more general utility, but I'm not sure how far it is now; I'm getting more up to speed with |
|
I don't think I made I'm also going to kick off a couple of issues — for BQ integration tests, and for our |
This was referenced Jul 27, 2022
Closed
|
@max-sixty Thanks! It looks good from the playground. I'll let you know if I run into any more issues. |
max-sixty
added a commit
that referenced
this pull request
Jul 29, 2022
* Fix BigQuery quoting Not sure this will be the end of it, but progress * .
max-sixty
added a commit
that referenced
this pull request
Jul 29, 2022
* Add double-bracket strings * Update prql-compiler/src/prql.pest * Remove unnecessary steps from GHAs (#805) * Remove unnecessary steps from GHAs * Remove version from mdbook-admonish install (#808) As dscribed in the comment, not worth the extra time to pin the version * Add section to contributing.md explaining our tests (#807) * Add section to contributing explaining our tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * . * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Release prql-java (#781) * rename java test package name * cross try * release conf * fix tests * update doc * profile adjustment * pre-commit * update developers Co-authored-by: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Co-authored-by: Maximilian Roos <m@maxroos.com> * Bump actions/setup-java from 1 to 3 (#810) Bumps [actions/setup-java](https://github.com/actions/setup-java) from 1 to 3. - [Release notes](https://github.com/actions/setup-java/releases) - [Commits](actions/setup-java@v1...v3) --- updated-dependencies: - dependency-name: actions/setup-java dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Improve GHA caching (#811) * Improve GHA caching Swap out actions-rs for @baptiste0928 crate * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Use default target for book build (#813) Now we no longer have the playground, no need to use wasm to test the book * Dialects for ident escaping (#809) * prql-macros (#814) * Bump rusqlite from 0.27.0 to 0.28.0 (#815) * Bump @testing-library/user-event from 14.2.1 to 14.2.3 in /playground (#816) * Remove incorrect link (#821) Removing this because it generates a 404, as per #818. Is there a way to link to a section on the homepage? Linking to prql-lang.org/index.html#Bindings doesn't seem to work either. * Bump @testing-library/user-event from 14.2.3 to 14.2.5 in /playground (#817) Bumps [@testing-library/user-event](https://github.com/testing-library/user-event) from 14.2.3 to 14.2.5. - [Release notes](https://github.com/testing-library/user-event/releases) - [Changelog](https://github.com/testing-library/user-event/blob/main/CHANGELOG.md) - [Commits](testing-library/user-event@v14.2.3...v14.2.5) --- updated-dependencies: - dependency-name: "@testing-library/user-event" dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add back link to list of bindings from FAQ (#823) * docs: link to list of bindings from faq * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * avoid setting empty id for most sections * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Add "copy to clipboard" button to playground for output SQL (#825) * add "copy to clipboard" to playground output sql * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Bump @testing-library/user-event from 14.2.5 to 14.2.6 in /playground (#824) Bumps [@testing-library/user-event](https://github.com/testing-library/user-event) from 14.2.5 to 14.2.6. - [Release notes](https://github.com/testing-library/user-event/releases) - [Changelog](https://github.com/testing-library/user-event/blob/main/CHANGELOG.md) - [Commits](testing-library/user-event@v14.2.5...v14.2.6) --- updated-dependencies: - dependency-name: "@testing-library/user-event" dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump terser from 5.13.1 to 5.14.2 in /playground (#830) Bumps [terser](https://github.com/terser/terser) from 5.13.1 to 5.14.2. - [Release notes](https://github.com/terser/terser/releases) - [Changelog](https://github.com/terser/terser/blob/master/CHANGELOG.md) - [Commits](https://github.com/terser/terser/commits) --- updated-dependencies: - dependency-name: terser dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add from_json to compiler (#829) * Add from_json to compiler * Update prql-compiler/src/lib.rs Co-authored-by: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Co-authored-by: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> * Bump insta from 1.15.0 to 1.16.0 (#832) Bumps [insta](https://github.com/mitsuhiko/insta) from 1.15.0 to 1.16.0. - [Release notes](https://github.com/mitsuhiko/insta/releases) - [Changelog](https://github.com/mitsuhiko/insta/blob/master/CHANGELOG.md) - [Commits](mitsuhiko/insta@1.15.0...1.16.0) --- updated-dependencies: - dependency-name: insta dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add test for from_json (#831) * Adding test for from_json * Formatting * Using better prql to test with * Fixes #748, tests for prql-js (#833) * For #748 , tests for javascript package * Bump @testing-library/user-event from 14.2.6 to 14.3.0 in /playground (#827) Bumps [@testing-library/user-event](https://github.com/testing-library/user-event) from 14.2.6 to 14.3.0. - [Release notes](https://github.com/testing-library/user-event/releases) - [Changelog](https://github.com/testing-library/user-event/blob/main/CHANGELOG.md) - [Commits](testing-library/user-event@v14.2.6...v14.3) --- updated-dependencies: - dependency-name: "@testing-library/user-event" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix some display trait bugs (#836) * fix some display trait bugs * convert single character string to char * replace find and is_some with contains * add snapshot test for display trait (#837) * add snapshot test for display trait * regenerate test * Fix quoting issues in idents (#838) * Fix quoting issues in idents * Update docs * Fixes #834, book/integrations for rust and python (#835) * Fixes #834, integrations for rust and python * Fix precedence issue (#839) * Fix precedence issue Not yet complete, but solves the immediate issue, and a nice framework to build off, thanks to @alijazerzen * Complete the BinOps * . * Fix book build (#840) The SQL wasn't being created in the book recently; and the mdbook uses an error as a value in its preprocessors, so this wasn't getting caught. Not sure what a better design is, beyond me checking the actual website rather than my local build (or mdbook improving their interface...) * Bump dependencies (#841) * Fix book build The SQL wasn't being created in the book recently; and the mdbook uses an error as a value in its preprocessors, so this wasn't getting caught. Not sure what a better design is, beyond me checking the actual website rather than my local build (or mdbook improving their interface...) * Bump dependencies To bust cache, since #840 compiles new dependencies without changing Cargo.lock * Attempt to silence codeql warning (#842) ref github/codeql#4850 * Update gitignore (#844) Don't include `.prql` files; consolidate `target` paths * Improve error message on multi-part ident in equijoins (#845) * Add release info to prql-macros crate (#846) We don't publish this separately; `cargo-release` needs this info * Add release info to prql-macros crate (#846) (#847) We don't publish this separately; `cargo-release` needs this info * Add cargo-release onto `task setup-dev` (#843) * Add cargo-release onto `task setup-dev` And remove deprecated task * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * (cargo-release) version 0.2.3 (#848) * Disable prql-java release workflow (#851) Until #850 is fixed. (TBC, no great stress here -- better to have started and rolled back than never to have started :) ) * Fix python release workflow (#849) * Fix python releaes workflow Really we should have a test release workflow running which does a dry-run, so we don't get these sorts of problems (which I'm sure was my fault tbc...) * Unique row number aliases (#853) * Adding row_number_id to ROW_NUMBER() SQL to make them unique * Adding tests for #826 * Fixing clippy * Testing raw flag to pass the deadlinks check * Apply suggestions from code review Try reverting raw link change Co-authored-by: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> * Only run check-links on modified files (#854) * Only run check-links on modified files And then run every day on all. This should reduce the issues we had in #853, not the first time we've had them. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Only install wasm-pack in CI when needed (#855) * Only install wasm-pack in CI when needed GHA builds are getting longer now, we're well past our 1 minute target... Though this isn't going to help much! * Add some workflows to track our compilation time (#856) * Add some workflows to track our compilation time It's getting quite long now — CI jobs take 90-150 seconds, almost all of which is compilation, even with a cache. Locally I'm seeing more like 20s after wiping `target`. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * . * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * . * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Update docs in ast_fold.rs (#857) * Precedence for unary operators and IS NULL (#859) * Test compilation time for all targets (#860) * Upgrade dependencies (#861) Busts cache so #860 will cache correctly * cli revamp (#863) * Reorganize top-level functions (#865) * Remove cached cargo-timings files in GHA (#864) * Remove cached cargo-timings files in GHA * Bump deps (#867) Another cache bust, this time for #864 * Fix links to stdlib.prql (#868) Weirdly the book printed an error, but passed?! https://github.com/prql/prql/runs/7530558870?check_suite_focus=true (this was from #1513, zero stress tbc) * Fix BigQuery quoting (#858) * Fix BigQuery quoting Not sure this will be the end of it, but progress * . * Bump insta from 1.16.0 to 1.17.0 (#870) Bumps [insta](https://github.com/mitsuhiko/insta) from 1.16.0 to 1.17.0. - [Release notes](https://github.com/mitsuhiko/insta/releases) - [Changelog](https://github.com/mitsuhiko/insta/blob/master/CHANGELOG.md) - [Commits](https://github.com/mitsuhiko/insta/commits) --- updated-dependencies: - dependency-name: insta dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Remove miscrate eprintln statement (#871) Guessing this was an oversight * Final fix of BQ quotes (hopefully) (#874) * Switch implicit `compile` tests to actual `compile` tests (#875) * Switch implicit `compile` tests to actual `compile` tests * Bump wasm-react-scripts from 5.0.2 to 5.0.3 in /playground (#876) Bumps [wasm-react-scripts](https://github.com/Cosmian/create-react-app/tree/HEAD/packages/react-scripts) from 5.0.2 to 5.0.3. - [Release notes](https://github.com/Cosmian/create-react-app/releases) - [Changelog](https://github.com/Cosmian/create-react-app/blob/main/CHANGELOG-1.x.md) - [Commits](https://github.com/Cosmian/create-react-app/commits/react-error-overlay@5.0.3/packages/react-scripts) --- updated-dependencies: - dependency-name: wasm-react-scripts dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * (cargo-release) version 0.2.4 (#877) * Only one workflow event per release (#878) * . Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Jie Han <11144133+doki23@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Aljaž Mur Eržen <aljazerzen@users.noreply.github.com> Co-authored-by: Marko Klopets <mklopets@gmail.com> Co-authored-by: Arrizal Amin <arrizalamin@gmail.com> Co-authored-by: charlie sanders <711385+charlie-sanders@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Not sure this will be the end of it, but progress