Skip to content

Add bound parameter support to fdb push writes#6015

Open
emelialei88 wants to merge 1 commit into
bloomberg:mainfrom
emelialei88:feat/fdb-push-write-bind-param
Open

Add bound parameter support to fdb push writes#6015
emelialei88 wants to merge 1 commit into
bloomberg:mainfrom
emelialei88:feat/fdb-push-write-bind-param

Conversation

@emelialei88

Copy link
Copy Markdown
Contributor

Summary

  • Remote push writes (INSERT/UPDATE/DELETE) now forward bound parameters to the remote database, matching the existing behavior for push reads
  • Adds test coverage for write operations with various bound parameter types (integer, real, cstring)

Test plan

  • Added INSERT, UPDATE, DELETE test cases with @bind parameters to fdb_push.test
  • Verified parameter types: CDB2_INTEGER, CDB2_REAL, CDB2_CSTRING

@emelialei88 emelialei88 force-pushed the feat/fdb-push-write-bind-param branch from 0b5ff3e to 7dac30d Compare June 9, 2026 20:35

@roborivers roborivers 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.

Cbuild submission: Error ⚠.
Regression testing: Success ✓.

The first 10 failing tests are:
sc_resume_logicalsc_generated **quarantined**
consumer_non_atomic_default_consumer_generated **quarantined**
reco-ddlk-sql [timeout] **quarantined**
sql_logfill_autodisable [timeout]

@emelialei88 emelialei88 marked this pull request as ready for review June 10, 2026 15:03

@dorinhogea dorinhogea left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please check:

free(push->remotedb);

It frees fdb_push object and its remotedb, but not the new param object that is leaked0

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>
@emelialei88 emelialei88 force-pushed the feat/fdb-push-write-bind-param branch from 7dac30d to fd257b6 Compare June 10, 2026 21:08
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.

3 participants