refactor: extract fuse compat test matrix into test_cases.yaml#19441
refactor: extract fuse compat test matrix into test_cases.yaml#19441dantengsky merged 24 commits intodatabendlabs:mainfrom
Conversation
github-actions
bot
added
the
pr-refactor
|
@codex review |
dantengsky
force-pushed
the
refactor/fuse-compatibility-check
branch
from
e8db300 to
2d0087f
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea465c7ffd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
dantengsky
force-pushed
the
refactor/fuse-compatibility-check
branch
from
7190601 to
60b0308
Compare
dantengsky
force-pushed
the
refactor/fuse-compatibility-check
branch
from
60b0308 to
e70e404
Compare
…cutes them The sqllogictest runner filters files by `.test` extension (tests/sqllogictests/src/main.rs:308). Without it, `--run_file` never matches and 0 tests are executed. This is a pre-existing bug on main that affects all compat_fuse cases.
Old query (1.2.46) does not support trailing commas in CREATE TABLE column definitions or INSERT VALUES. This was never caught because the test files were never actually executed (missing .test extension).
Current query expands ALL into individual privilege names (CREATE,SELECT,INSERT,...) instead of showing ALL.
rbac/ reverted to pre-54fceaa6b0 content compatible with writer 1.2.311. rbac_all/ preserves the expanded test (procedure/sequence/connection/ warehouse) with writer 1.2.833 which supports all these features.
dantengsky
force-pushed
the
refactor/fuse-compatibility-check
branch
from
5eea9b3 to
afc4f7d
Compare
PR databendlabs#18730 wrote "GRANT access sequence on procedure" which is syntactically wrong. Should be "GRANT access procedure on procedure". Never caught because the test was never executed.
rbac -> rbac_v1_2_311 (compatible with writer 1.2.311/1.2.318) rbac_all -> rbac_current (requires current query features)
- Add -c advice.detachedHead=false to git clone in git_partial_clone() - Capture --cmd ver output to suppress backtrace from old binaries
- Add context message before/after version checks - Only print the "version:" line from --cmd ver output - Use "checking metasrv/query" prefix for clarity
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6099bd66b7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
2 participants
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
Extract the 8 hardcoded
python3invocations inaction.ymlinto a declarativetest_cases.yamlfile, and fix a pre-existing bug where all compat_fuse sqllogictest files have never been executed since 2025-02-14.Refactoring: test matrix extraction
test_cases.yaml: declarative test matrix with commentstest_compat_fuse.py: add--run-all/--dry-run, use PyYAML for parsingaction.yml: 8 lines → 1 linepython3 ... --run-allBug fix: compat_fuse sqllogictest silently broken since 2025-02-14
Root cause:
54fceaa6b0(2025-02-14, PR #17449) added a.testextension filter to the sqllogictest runner. All compat_fuse test files (fuse_compat_write,fuse_compat_read) lack the.testextension. From that point on, the runner silently skipped them — CI reported green withRun all tests[0](0 tests executed).Subsequent PRs added incompatible SQL without noticing. After the filter silenced execution, multiple PRs added SQL that older writer versions don't support.
Fixes:
.testextension so the runner finds them--run_fileparameter to match new filenamesbase/fuse_compat_write.test(incompatible with writer 1.2.46)revoke/fuse_compat_read.testexpected output forshow grants(current query expandsALLinto individual privilege names)RBAC test suite changes
After the
.testextension filter silenced test execution (2025-02-14), PRs #17262, #18382, #18423, #18730 added procedure/sequence/connection/warehouse SQL torbac/fuse_compat_writethat writer versions 1.2.311/1.2.318 don't support. The original rbac suite (from PR #14393, writer 1.2.306, bumped to 1.2.311 in PR #18500) was basic role/grant/ownership tests only.Split into two version-aware suites:
rbac_v1_2_311/— original basic rbac test (reverted to pre-54fceaa6b0content), used by writer 1.2.311 and 1.2.318rbac_current/— full rbac test with procedure/sequence/connection/warehouse, writer 1.2.833Typo fix in
rbac_current/fuse_compat_read.test:GRANT access sequence on procedure p1(int)→GRANT access procedure on procedure p1(int). The original SQL (from PR #18730) is syntactically invalid — afterGRANT access sequence on, the parser expects a sequence name, not the reserved keywordprocedure. Likely a copy-paste error from the line above (GRANT access sequence on sequence seq2). Changed toaccess procedureto match the object type.Removed: V0 compat tests
The
compat-stateless/01_flashback/,compat-stateless/02_flashback_v3_to_v4/and their logictest suites (01_meta_compression/,02_meta_compression_v3_to_v4/) were removed. These covered fuse table format version V0 compatibility (writer 1.0.56 and 1.1.30), already dropped inea91e06b93(PR #14818).Tests
test_compat_fuse— verifies all 10 cases actually execute sqllogictest files (was 0 tests before)Type of change
This change is