Skip to content

ADBDEV-10570: Fix broken assertion for Clickhouse connector - #59

Open
riogor wants to merge 1 commit into
mainfrom
ADBDEV-10570
Open

ADBDEV-10570: Fix broken assertion for Clickhouse connector#59
riogor wants to merge 1 commit into
mainfrom
ADBDEV-10570

Conversation

@riogor

@riogor riogor commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

An assertion, checking for fdw_private being integer list to get metadata queue id
fails if fdw_private is not an integer list.
This happens when used with FDW that doesn't support metadata queue mechanism.
PXF doesn't check for metadata support at queue id retrieval phase.

Patch covers queue id retrieval with check for FDW metadata support.

No tests because a possible test sends a malformed fdw_private, which
means that connector was done incorrectly, while pxf assumes otherwise.

Ticket: ADBDEV-10570

Co-authored-by: Denis Kovalev d.kovalev@arenadata.io

Comment thread fdw/pxf_fdw.c
if (fdw_private != NULL)
if (IsExtCommitMetadata(options) && fdw_private != NULL)
{
Assert(list_length(fdw_private) == 1);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would also add Assert(IsIntegerList(fdw_private));

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Is assert in list_nth_int not enough?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Its not in our code, and error is bit tricky and require stack analyze to get the reason. Here we would get an assert message related to this line and its much easier to track it down that the problem in fdw_private.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

IsIntegerList macro exists only in list.c and is not available outside. Copying macro code doesn't look good either.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

right

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