Skip to content

feat(sync-service): add Postgres concat(VARIADIC text) to shape subset eval#4281

Merged
robacourt merged 6 commits into
electric-sql:mainfrom
abegehr:abegehr/impl-concat
May 11, 2026
Merged

feat(sync-service): add Postgres concat(VARIADIC text) to shape subset eval#4281
robacourt merged 6 commits into
electric-sql:mainfrom
abegehr:abegehr/impl-concat

Conversation

@abegehr
Copy link
Copy Markdown
Contributor

@abegehr abegehr commented May 6, 2026

Register concat with NULL-skipping semantics (empty string when all args are NULL), matching text concat behavior distinct from || with NULL.

Tests cover literals, refs, ilike(concat(...)), contrast with ||, and document that concat() is unsupported (no concat/0 overload).

abegehr added 3 commits May 6, 2026 20:02
…t eval

Register concat with NULL-skipping semantics (empty string when all args
are NULL), matching text concat behavior distinct from || with NULL.

Tests cover literals, refs, ilike(concat(...)), contrast with ||, and
document that concat() is unsupported (no concat/0 overload).
Enhance SQL generator to support variadic concat function, allowing concatenation of multiple elements wrapped in an array. Update tests to verify the correct SQL output for concat with array arguments, ensuring compatibility with existing functionality.
@abegehr abegehr marked this pull request as ready for review May 6, 2026 17:23
Copy link
Copy Markdown
Contributor

@robacourt robacourt left a comment

Choose a reason for hiding this comment

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

Looking good! Please could you add some tests for some of the other edge cases mentioned here: https://chatgpt.com/share/69fb490a-4c2c-8327-a6a2-74f730c95ea9 e.g. date types, variadic arrays, char(n). It's fine to just return an error when parsing so that we don't support them, but if we do parse then successfully we should handle these cases correctly.

Add tests to improve error handling for the concat function in the SQL parser. Specifically, reject concat with non-text date arguments, keep explicit variadic concat array forms unsupported, and reject concat with char(n) until bpchar support is implemented. These changes ensure better validation and user feedback for unsupported operations.
@abegehr abegehr requested a review from robacourt May 9, 2026 12:58
@abegehr
Copy link
Copy Markdown
Contributor Author

abegehr commented May 9, 2026

added three more tests for edge-cases

@netlify
Copy link
Copy Markdown

netlify Bot commented May 9, 2026

Deploy Preview for electric-next ready!

Name Link
🔨 Latest commit e6b9d25
🔍 Latest deploy log https://app.netlify.com/projects/electric-next/deploys/69ff2f5e051aca000818bede
😎 Deploy Preview https://deploy-preview-4281--electric-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Comment thread .changeset/moody-maps-approve.md Outdated
Copy link
Copy Markdown
Contributor

@robacourt robacourt left a comment

Choose a reason for hiding this comment

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

Great work! Approved!

@codecov
Copy link
Copy Markdown

codecov Bot commented May 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.60%. Comparing base (686ebdb) to head (d6eeeff).
⚠️ Report is 7 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (686ebdb) and HEAD (d6eeeff). Click for more details.

HEAD has 7 uploads less than BASE
Flag BASE (686ebdb) HEAD (d6eeeff)
unit-tests 10 8
typescript 10 8
packages/react-hooks 1 0
packages/experimental 1 0
packages/typescript-client 1 0
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4281      +/-   ##
==========================================
- Coverage   63.06%   55.60%   -7.47%     
==========================================
  Files         184      233      +49     
  Lines       19932    22380    +2448     
  Branches     4956     5853     +897     
==========================================
- Hits        12571    12444     -127     
- Misses       7358     9930    +2572     
- Partials        3        6       +3     
Flag Coverage Δ
packages/agents 67.80% <ø> (+6.60%) ⬆️
packages/agents-mcp 77.19% <ø> (?)
packages/agents-runtime 80.10% <ø> (+0.86%) ⬆️
packages/agents-server 69.27% <ø> (+3.23%) ⬆️
packages/agents-server-ui 5.31% <ø> (+3.66%) ⬆️
packages/electric-ax 38.04% <ø> (-0.55%) ⬇️
packages/experimental ?
packages/react-hooks ?
packages/start 82.83% <ø> (ø)
packages/typescript-client ?
packages/y-electric 56.05% <ø> (ø)
typescript 55.60% <ø> (-7.47%) ⬇️
unit-tests 55.60% <ø> (-7.47%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@robacourt robacourt merged commit f460584 into electric-sql:main May 11, 2026
45 of 46 checks passed
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.

2 participants