Skip to content

Conversation

@Mandukhai-Alimaa
Copy link
Contributor

This PR contains:

There was a small change made in the c/driver/postgresql/result_helper.cc due to the segfault when the number of query parameters doesn't match the number of result columns. The test that triggered the bug was calling get_parameter_schema() on SELECT 1 + $1 + $2 .

This query has 2 parameters but returns only 1 result column (the computed sum). The code was incorrectly using PQfname(result_, i) to retrieve parameter names, but PQfname() actually retrieves result column names from the result set. When iterating through parameters, the first iteration (i=0) would work fine accessing column 0, but the second iteration (i=1) would access an out-of-range column. PQfname() returns NULL for out-of-range column numbers, and passing this NULL pointer to AppendChild() as a C string causes the segfault.

Comment on lines +52 to +53
# PostgreSQL returns -1 for CREATE TABLE
assert rows_affected == -1
Copy link
Member

Choose a reason for hiding this comment

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

If multiple vendors do this, we should probably add a flag to the test itself instead of requiring overriding this test every time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created an issue for it in the validation repo (adbc-drivers/validation#140). I will leave a TODO in the code for now and come back and fix it after I implement a separate flag in the validation repo.

Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

Thanks!

@lidavidm lidavidm merged commit 4a3b8eb into apache:main Dec 29, 2025
74 of 75 checks passed
@lidavidm
Copy link
Member

@Mandukhai-Alimaa can you file issues to fix any skipped tests?

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