Repository navigation
Conversation
Contributor
|
Preview root: https://posit-dev.github.io/commons/pr-437/ Python site preview: https://posit-dev.github.io/commons/pr-437/py/ Built from the latest commit on this branch. The R links in it point at the published R site, which no pull request rebuilds. |
jat255
force-pushed
the
jat255/8kf5-catalog-merge-error-classes
branch
from
October 21, 2026 22:03
2528d72 to
2e33b0e
Compare
jat255
force-pushed
the
jat255/5hfw-definition-binding-fixture
branch
from
October 21, 2026 22:03
8fca4c4 to
95aed4e
Compare
Definition binding was specified only in the Python suite, though both packages implement it. definition-binding.json now pins it: authored columns lowered with the warehouse's spelling on Snowflake and Databricks, marker reservation through a column and through a COLUMNS(...) selection, and the two refusals, as slugs mapped to a class in each runner. A COLUMNS(...) selection from a pattern or from * now refuses a column the relation lacks, as one written out by name already did. Python dropped such a column instead, so the two packages disagreed on a dictionary documenting a column the warehouse no longer has. R's always-true strict flag goes with it, since no caller binds any other way. Both runners share the catalog-merge hydration, moved into a test helper.
A dictionary with several faults got a different refusal from each package: Python checked every table's match up front and metric grain last, while R checks grain first and then each table in authored order, its match before its columns. Python now follows that order, its grain refusal gets a class in both packages, and three definition-binding cases pin the order. The definition error classes are re-exported from commons._definitions, as the catalog ones are from commons._catalog, and both runners name the case when a refusal is missing or of the wrong class.
jat255
force-pushed
the
jat255/8kf5-catalog-merge-error-classes
branch
from
October 22, 2026 02:46
2e33b0e to
97bc0e6
Compare
jat255
force-pushed
the
jat255/5hfw-definition-binding-fixture
branch
from
October 22, 2026 02:46
95aed4e to
073aa85
Compare
This branch has not been deployed
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
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.
This PR adds a shared fixture,
definition-binding.json. It pins how both packages bind a governed definition to the column names a warehouse catalog reports, and which error each package raises when it cannot. Stacked on #436.Python behaviour change. Since py-v0.1.0b1,
COLUMNS(pattern)andCOLUMNS(*)dropped an authored column that the warehouse relation does not have. They now refuse it, as a selection written out by name already did, and as R always has (_compile.py). This needs a line in the nextpy-v*release notes. Python also now checks a dictionary with several faults in the same order as R, so both packages report the same fault.R changes
R's binding results do not change. Its three binding refusals now have condition classes, listed in NEWS: a metric that mixes row and aggregate grain, a table that matches no relation, and a column the relation lacks. The
strictfield in the catalog's definition bindings is gone fromdefinition-compile.R. It was alwaysTRUE, so nothing behaves differently. One test intest-definition-compile.Ris now a fixture case. The catalog-merge runner's setup code moved intohelper-catalog-rows.R, so both runners share it.Agent-written details
COLUMNS(...)selection that matches no column compiles to empty SQL in both packages, with or without bindings. That bug is older than this PR and is out of scope here.pyrefly check src testsand the full pytest run pass. The Rdevtools::test()run passes. The fullR CMD checkwas not run. Each fixture case was checked to fail against code with the behaviour it pins removed, including the old Python selection rule and the old Python check order.