Skip to content

fix(sql): fix an issue where the DECLAREd symbols weren't honored when parsing function arguments#6700

Merged
bluestreak01 merged 5 commits intomasterfrom
mt_declare-type-conflict
Jan 23, 2026
Merged

fix(sql): fix an issue where the DECLAREd symbols weren't honored when parsing function arguments#6700
bluestreak01 merged 5 commits intomasterfrom
mt_declare-type-conflict

Conversation

@mtopolnik
Copy link
Copy Markdown
Contributor

@mtopolnik mtopolnik commented Jan 23, 2026

Fixes #6547

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 23, 2026 •
edited
Loading

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Modified SqlParser to pass model declarations when parsing expressions in the FROM clause, enabling declared variables to be resolved. Added and reorganized test cases for DECLARE functionality, including substitution in function arguments within FROM clauses.

Changes

Cohort / File(s) Summary
Parser logic
core/src/main/java/io/questdb/griffin/SqlParser.java		 Modified parseSelectFrom to pass model.getDecls() parameter to expr() call, enabling declaration lookup during FROM clause expression parsing
Test coverage
core/src/test/java/io/questdb/test/griffin/DeclareTest.java		 Added 6 new test methods for DECLARE OVERRIDABLE functionality and FROM clause variable substitution; reorganized test placement to avoid duplication (46 lines added, 33 removed)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Suggested labels

Bug, SQL

Suggested reviewers

  • bluestreak01
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: fixing DECLAREd symbols not being honored when parsing function arguments.
Description check ✅ Passed The description references the linked issue #6547, which relates directly to the DECLARE symbol handling that the code changes address.
Linked Issues check ✅ Passed The code changes directly address issue #6547 by passing SQL declarations to parseSelectFrom [SqlParser.java], enabling declared variables to work correctly in function arguments within FROM clauses, and comprehensive tests validate this functionality [DeclareTest.java].
Out of Scope Changes check ✅ Passed All changes are scope-appropriate: the SqlParser modification enables declaration passing for FROM clause parsing, and the test additions validate DECLARE functionality in function arguments as required by issue #6547.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mtopolnik
Copy link
Copy Markdown
Contributor Author

mtopolnik commented Jan 23, 2026

@CodeRabbit Please review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 23, 2026

@mtopolnik, I'll review the changes in this pull request right away.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@glasstiger
Copy link
Copy Markdown
Contributor

glasstiger commented Jan 23, 2026

[PR Coverage check]

😍 pass : 1 / 1 (100.00%)

file detail

path covered line new line coverage
🔵 io/questdb/griffin/SqlParser.java 1 1 100.00%

@bluestreak01 bluestreak01 merged commit b29ca53 into master Jan 23, 2026
43 checks passed
@bluestreak01 bluestreak01 deleted the mt_declare-type-conflict branch January 23, 2026 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bluestreak01 bluestreak01 bluestreak01 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

declare substitution inconsistent with implicit casts and constant folding

3 participants

@mtopolnik @glasstiger @bluestreak01