Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use SPITupleTable->numvals when available #1037

Merged

Conversation

yrashk
Copy link
Contributor

@yrashk yrashk commented Feb 8, 2023

Starting from PostgreSQL, SPI tuple table contains number of rows returned in numvals, with SPI_processed still available for "historical reasons":

"The fields tupdesc, vals, and numvals can be used by SPI callers; the remaining fields are internal. vals is an array of pointers to rows. The number of rows is given by numvals (for somewhat historical reasons, this count is also returned in SPI_processed)."

(https://www.postgresql.org/docs/current/spi-spi-execute.html)

Also:

"Some utility commands (COPY, CREATE TABLE AS) don't return a row set, so SPI_tuptable is NULL, but they still return the number of rows processed in SPI_processed."

So we're following this logic.

This is to ensure we're not relying on "historical" API going forward.

Starting from PostgreSQL, SPI tuple table contains number of rows
returned in `numvals`, with `SPI_processed` still available for
"historical reasons":

"The fields tupdesc, vals, and numvals can be used by SPI callers; the
remaining fields are internal. vals is an array of pointers to rows. The
number of rows is given by numvals (for somewhat historical reasons,
this count is also returned in SPI_processed)."

(https://www.postgresql.org/docs/current/spi-spi-execute.html)

Also:

"Some utility commands (COPY, CREATE TABLE AS) don't return a row set,
so SPI_tuptable is NULL, but they still return the number of rows
processed in SPI_processed."

So we're following this logic.

This is to ensure we're not relying on "historical" API going forward.
pgx/src/spi.rs Show resolved Hide resolved
@eeeebbbbrrrr eeeebbbbrrrr changed the base branch from master to develop February 8, 2023 19:29
@eeeebbbbrrrr
Copy link
Contributor

I fixed it but remember we like PRs against develop. ;)

@yrashk
Copy link
Contributor Author

yrashk commented Feb 8, 2023

I fixed it but remember we like PRs against develop. ;)

Sorry about that! My colds-induced brain fog is still here.

@eeeebbbbrrrr
Copy link
Contributor

I'm re-running CI for this. Don't remember why it failed, and the logs have rolled off. haha. Once CI goes green I'll merge. This is fine.

@eeeebbbbrrrr eeeebbbbrrrr merged commit 05d8b35 into pgcentralfoundation:develop Feb 16, 2023
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.

None yet

2 participants