Skip to content

fix(pgwire): support UUID bind variables in JIT filters#6413

Merged
bluestreak01 merged 5 commits intomasterfrom
jh_uuid_binding_variables
Nov 19, 2025
Merged

fix(pgwire): support UUID bind variables in JIT filters#6413
bluestreak01 merged 5 commits intomasterfrom
jh_uuid_binding_variables

Conversation

@jerrinot
Copy link
Copy Markdown
Contributor

@jerrinot jerrinot commented Nov 19, 2025 •
edited by ideoma
Loading

This change adds support for UUID binding variables within JIT-compiled filters.
Before this fix, PGWire queries eligible for JIT compilation that used a UUID parameter
would fail with: ERROR: unsupported bind variable type: UUID

Workaround before this fix:
If you encounter this error on an unpatched version, use explicit casting
to disable JIT:
Instead of: SELECT * FROM table WHERE uuid = $1;
Use: SELECT * FROM table WHERE uuid = $1::uuid;

Fixes #6408

@jerrinot
fix(pgwire): support UUID bind variables in JIT filters
39e081d 
This change adds support for UUID binding variables within JIT-compiled filters.
Before this fix, PGWire queries eligible for JIT compilation that used a UUID parameter
would fail with: 'ERROR: unsupported bind variable type: UUID'

Workaround before this fix:
If you encounter this error on an unpatched version, use explicit casting
to bypass JIT compilation:
Instead of: SELECT * FROM table WHERE uuid = $1;
Use: SELECT * FROM table WHERE uuid = $1::uuid;
@jerrinot
look ma, no concat!
b2eec7c
@jerrinot jerrinot added Bug Incorrect or unexpected behavior Postgres Wire Issues or changes relating to Postgres wire protocol Java Improvements that update Java code labels Nov 19, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 19, 2025 •
edited
Loading

Walkthrough

Adds UUID (128-bit) type binding support to the AVM/JIT bind-variable writing path in AsyncJitFilteredRecordCursorFactory by retrieving UUID values via getLong128Lo/Hi and writing them with putLong128(lo, hi). Includes test coverage verifying UUID binding functionality across bind variables, compiled filters, and PostgreSQL wire protocol contexts.

Changes

Cohort / File(s) Summary
UUID Binding Implementation
core/src/main/java/io/questdb/griffin/engine/table/AsyncJitFilteredRecordCursorFactory.java		 Adds support for UUID type in the bind-variable writing path. When bind variable type is UUID, retrieves 128-bit value via getLong128Lo/Hi and writes it via putLong128(lo, hi) to bindVarMemory. Extends error handling for unsupported types.
UUID Binding Tests
core/src/test/java/io/questdb/test/cutlass/pgwire/PGJobContextTest.java		 Adds testUuidBindingVariableInFilter() test that verifies UUID binding with parallel filter execution, creates table with UUID column, inserts data, binds UUID parameter, and asserts single-row result.
Compiled Filter Tests
core/src/test/java/io/questdb/test/griffin/CompiledFilterTest.java		 Adds testUuid() test that constructs table with UUID column, binds UUID value via BindVariableServiceImpl, executes query comparing UUID column to bound value, and verifies results both with and without JIT compilation. Imports Uuid utility.
Bind Variables Tests
core/src/test/java/io/questdb/test/griffin/engine/functions/bind/BindVariablesTest.java		 Adds testUuid() test that creates table with UUID column, constructs UUID using Uuid utility, binds via setUuid, inserts using bind parameter ($1), and asserts stored value matches expected UUID string. Imports Uuid class.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10-15 minutes

  • Primary focus: AsyncJitFilteredRecordCursorFactory.java — Review the UUID case addition to bind-variable handling logic
  • Secondary focus: Test methods follow existing patterns but verify end-to-end UUID binding across multiple execution contexts (JIT, PostgreSQL wire, compiled filters, and bind variables)

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding UUID bind variable support to JIT filters in PGWire, which directly matches the core modification in AsyncJitFilteredRecordCursorFactory.java.
Description check ✅ Passed The pull request description clearly relates to the changeset, describing the addition of UUID binding variable support in JIT-compiled filters with specific error context and workaround.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jh_uuid_binding_variables

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

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.

@jerrinot
Copy link
Copy Markdown
Contributor Author

jerrinot commented Nov 19, 2025

@coderabbitai pls review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 19, 2025

@jerrinot: I'll review the changes now.

I notice that the PR title needs to be updated to match the required format: type(subType): description. Based on the PR objectives mentioning "UUID binding variables", this appears to be a feature addition, so the title might be something like:

feat(sql): add UUID binding variables

or

feat(core): add UUID binding variables

depending on which component is most appropriate for this change.

I'll proceed with the review of the code changes.

✅ 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.

@jerrinot jerrinot changed the title Jh UUID binding variables fix(pgwire): support UUID bind variables in JIT filters Nov 19, 2025
coderabbitai[bot]
coderabbitai bot reviewed Nov 19, 2025
View reviewed changes
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
core/src/test/java/io/questdb/test/griffin/CompiledFilterTest.java (1)

553-577: LGTM! UUID binding test is well-structured.

The test correctly exercises UUID bind variable functionality in the compiled filter path, following the established pattern from testAllBindVariableTypes(). It properly validates both normal execution and JIT compilation.

For future enhancement, consider adding test coverage for:

  • Null UUID values
  • Named bind variables (e.g., :uuid)
  • Multiple UUID comparisons in a single query
core/src/test/java/io/questdb/test/griffin/engine/functions/bind/BindVariablesTest.java (1)

894-910: LGTM! UUID binding and insertion test is correct.

The test properly validates UUID bind variable functionality for the insertion path, following the pattern established by testIPv4() above it. The use of getFunction(0) before setUuid() is consistent with existing bind variable tests.

For comprehensive coverage, consider adding tests for:

  • Null UUID handling
  • Named bind variables (e.g., :uuid)
  • Bind variable reuse with different UUID values
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e88abf7 and b2eec7c.

📒 Files selected for processing (4)
  • core/src/main/java/io/questdb/griffin/engine/table/AsyncJitFilteredRecordCursorFactory.java (1 hunks)
  • core/src/test/java/io/questdb/test/cutlass/pgwire/PGJobContextTest.java (1 hunks)
  • core/src/test/java/io/questdb/test/griffin/CompiledFilterTest.java (2 hunks)
  • core/src/test/java/io/questdb/test/griffin/engine/functions/bind/BindVariablesTest.java (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (36)
  • GitHub Check: build
  • GitHub Check: New pull request (SelfHosted Running tests with cover on linux-other)
  • GitHub Check: New pull request (SelfHosted Running tests with cover on linux-pgwire)
  • GitHub Check: New pull request (SelfHosted Running tests with cover on linux-cairo-sub)
  • GitHub Check: New pull request (SelfHosted Running tests with cover on linux-cairo-root)
  • GitHub Check: New pull request (SelfHosted Running tests with cover on linux-fuzz2)
  • GitHub Check: New pull request (SelfHosted Running tests with cover on linux-fuzz1)
  • GitHub Check: New pull request (SelfHosted Running tests with cover on linux-griffin-sub)
  • GitHub Check: New pull request (SelfHosted Other tests on linux-x64-zfs)
  • GitHub Check: New pull request (Rust Test and Lint on linux-jdk17)
  • GitHub Check: New pull request (SelfHosted Other tests on linux-x86-graal)
  • GitHub Check: New pull request (SelfHosted Running tests with cover on linux-griffin-root)
  • GitHub Check: New pull request (Hosted Running tests on windows-other-2)
  • GitHub Check: New pull request (Hosted Running tests on windows-other-1)
  • GitHub Check: New pull request (Hosted Running tests on windows-pgwire)
  • GitHub Check: New pull request (Hosted Running tests on windows-cairo-2)
  • GitHub Check: New pull request (Hosted Running tests on windows-cairo-1)
  • GitHub Check: New pull request (Hosted Running tests on windows-fuzz2)
  • GitHub Check: New pull request (Hosted Running tests on windows-fuzz1)
  • GitHub Check: New pull request (Hosted Running tests on windows-griffin-sub)
  • GitHub Check: New pull request (Hosted Running tests on windows-griffin-base)
  • GitHub Check: New pull request (Hosted Running tests on mac-other)
  • GitHub Check: New pull request (Hosted Running tests on mac-pgwire)
  • GitHub Check: New pull request (Hosted Running tests on mac-cairo-fuzz)
  • GitHub Check: New pull request (Hosted Running tests on mac-cairo)
  • GitHub Check: New pull request (SelfHosted Cairo tests on linux-x64-zfs)
  • GitHub Check: New pull request (SelfHosted Cairo tests on linux-arm64)
  • GitHub Check: New pull request (Hosted Running tests on mac-griffin)
  • GitHub Check: New pull request (SelfHosted Other tests on linux-arm64)
  • GitHub Check: New pull request (SelfHosted Griffin tests on linux-x86-graal)
  • GitHub Check: New pull request (SelfHosted Griffin tests on linux-x64-zfs)
  • GitHub Check: New pull request (SelfHosted Griffin tests on linux-arm64)
  • GitHub Check: New pull request (SelfHosted Cairo tests on linux-x86-graal)
  • GitHub Check: New pull request (Check Changes Check changes)
  • GitHub Check: build
  • GitHub Check: build
🔇 Additional comments (3)
core/src/main/java/io/questdb/griffin/engine/table/AsyncJitFilteredRecordCursorFactory.java (1)

418-422: LGTM! UUID binding support correctly implemented.

The implementation correctly handles UUID as a 128-bit value by retrieving the low and high parts and writing them to bind variable memory. This follows the established pattern for other types in the switch statement and completes the bind variable type support for UUID.

core/src/test/java/io/questdb/test/griffin/CompiledFilterTest.java (1)

38-38: LGTM! Uuid import added for new test.

The import is correctly placed alphabetically and supports the new UUID binding test.

core/src/test/java/io/questdb/test/griffin/engine/functions/bind/BindVariablesTest.java (1)

62-62: LGTM! Uuid import added for new test.

The import is correctly placed alphabetically and supports the new UUID binding test.

@glasstiger
Copy link
Copy Markdown
Contributor

glasstiger commented Nov 19, 2025

[PR Coverage check]

😍 pass : 4 / 4 (100.00%)

file detail

path covered line new line coverage
🔵 io/questdb/griffin/engine/table/AsyncJitFilteredRecordCursorFactory.java 4 4 100.00%

@bluestreak01 bluestreak01 merged commit ac2353c into master Nov 19, 2025
42 checks passed
@bluestreak01 bluestreak01 deleted the jh_uuid_binding_variables branch November 19, 2025 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@ideoma ideoma ideoma approved these changes

Assignees

No one assigned

Labels

Bug Incorrect or unexpected behavior Java Improvements that update Java code Postgres Wire Issues or changes relating to Postgres wire protocol

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Error when using UUID bind variable

4 participants

@jerrinot @glasstiger @ideoma @bluestreak01