Skip to content

fix(test): update pgsql digest fixtures for typecast trailing-space behavior#5804

Closed
renecannao wants to merge 2 commits into
v3.0from
fix/5755-followup-typecast-digest-fixtures
Closed

fix(test): update pgsql digest fixtures for typecast trailing-space behavior#5804
renecannao wants to merge 2 commits into
v3.0from
fix/5755-followup-typecast-digest-fixtures

Conversation

@renecannao

@renecannao renecannao commented May 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Updates 3 expected-digest test cases in tokenizer_payloads/pgsql_regular_tokenizer_digests.hjson to match the corrected tokenizer behavior introduced by PR #5755 (commit e6bef5c58), which intentionally preserves the trailing space after a PG typecast unless the next non-space char is a modifier ( or array bracket [.

Before this fix, pgsql-query_digests_stages_test-t fails deterministically in the legacy-g4 TAP group on v3.0 HEAD with 9 not-ok assertions across ::json ->, ::jsonb @>, and ::money + shapes.

Failures fixed

Query STAGE 1 actual (new tokenizer) STAGE 1 expected (was)
select '...'::json -> 'key' select ? -> ? select ?-> ?
select '...'::jsonb @> '...' select ? @> ? select ?@> ?
select '...'::money + '...'::money select ? + ? select ?+ ?

For -> and +, only STAGE 1 changes — STAGE 2-4's whitespace-collapse step still glues ?-> and ?+ for those operators. For @>, none of the stages collapse the space, so all four expectations move.

Test plan

Summary by CodeRabbit

  • Tests
    • Updated PostgreSQL tokenizer validation to reflect refined operator spacing handling in JSON and MONEY type operations.

Review Change Stack

…ehavior

PR #5755 (commit e6bef5c) fixed `process_pg_typecast()` in
`lib/pgsql_tokenizer.cpp` so that the trailing space after a PG
typecast is preserved unless the next non-space character is a
modifier `(` or array bracket `[`.  The commit message documented
this as an intentional behaviour change:

    "The new lookahead-conditional skip preserves the trailing
     space unless the very next non-space character is a modifier
     `(` or array bracket `[`."

Three test cases in `tokenizer_payloads/pgsql_regular_tokenizer_digests.hjson`
hardcoded expected digests against the OLD (buggy) tokenizer output,
where the space-eating bug glued the preceding literal to the
following operator.  With the fix in place those expectations
mismatch, making `pgsql-query_digests_stages_test-t` fail
deterministically in the legacy-g4 TAP group on v3.0 HEAD:

  * `::json  ->  'key'`  STAGE 1: actual `select ? -> ?`   vs old expected `select ?-> ?`
  * `::jsonb @>  ...`    STAGE 1-4: actual `select ? @> ?` vs old expected `select ?@> ?`
  * `::money +  ...`     STAGE 1: actual `select ? + ?`    vs old expected `select ?+ ?`

For `->` and `+`, only STAGE 1 changes — STAGE 2-4's existing
whitespace-collapse step still glues `?->` and `?+` for those
operators, so `s2`/`s3`/`s4` expectations stay as-is.  For `@>`,
none of the stages collapse the space, so all four expectations
move.

The new actual digest output is the more correct one — it mirrors
the existing MySQL tokenizer's behaviour for the same shape, where
`'literal' + 'literal'` already produces `? + ?` at STAGE 1.  This
commit only updates the fixture; no tokenizer or test source
changes.

Verified locally against the existing test binary (built from
v3.0 HEAD, includes the new tokenizer behaviour):

    ./pgsql-query_digests_stages_test-t  →  734/734 ok, RC=0

The 9 previously-failing assertions (#381, #385, #389-#392, #592,
#596, #600) all flip to ok with the updated fixture.
@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 65f4891e-014e-4dec-afb9-037ed547fc47

📥 Commits

Reviewing files that changed from the base of the PR and between 9719d4a and 1b5fd4d.

📒 Files selected for processing (1)
  • test/tap/tests/tokenizer_payloads/pgsql_regular_tokenizer_digests.hjson
📜 Recent review details
⏰ 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). (1)
  • GitHub Check: run / trigger
🔇 Additional comments (1)
test/tap/tests/tokenizer_payloads/pgsql_regular_tokenizer_digests.hjson (1)

281-281: LGTM!

Also applies to: 291-294, 659-659


📝 Walkthrough

Walkthrough

This PR updates PostgreSQL tokenizer test fixture expectations to reflect consistent operator spacing. Three operator cases are updated: JSON ->, JSONB @>, and MONEY + operators now expect spaces around them in the tokenizer digest output.

Changes

Operator Spacing Test Updates

Layer / File(s) Summary
Operator spacing in tokenizer digest expectations
test/tap/tests/tokenizer_payloads/pgsql_regular_tokenizer_digests.hjson
Expected tokenizer digests are updated across three test cases to include consistent spacing around operators: -> in JSON tests (line 281), @> in JSONB containment tests (lines 291–294), and + in MONEY type tests (line 659).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 The tokenizer hops with spacing so clean,
Operators wrapped in spaces pristine,
JSON, JSONB, and Money aligned,
With consistency of the most refined kind! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: updating test fixture expectations for PostgreSQL tokenizer behavior related to typecast trailing-space handling.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/5755-followup-typecast-digest-fixtures

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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the expected tokenizer digests in pgsql_regular_tokenizer_digests.hjson by adding spaces around various SQL operators, such as ->, @>, and +, in the test payloads. I have no feedback to provide as there were no review comments.

@sonarqubecloud

Copy link
Copy Markdown

@renecannao

Copy link
Copy Markdown
Contributor Author

Superseded by #5805 — both commits combined onto a single branch so CI runs once instead of twice. The cluster-1 commit (fix(test): update pgsql digest fixtures for typecast trailing-space behavior) is now e668a9dda on top of the ParserSQL 1.0.3 bump in #5805.

@renecannao renecannao closed this May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant