pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpg-migrator - pg_migrator: Document limitations.
13+ messages / 2 participants
[nested] [flat]
* pg-migrator - pg_migrator: Document limitations.
@ 2009-06-01 21:47 User Bmomjian <bmomjian@pgfoundry.org>
0 siblings, 0 replies; 13+ messages in thread
From: bmomjian@pgfoundry.org @ 2009-06-01 21:47 UTC (permalink / raw)
To: pgsql-committers
Log Message:
-----------
Document limitations.
Modified Files:
--------------
pg_migrator:
README (r1.21 -> r1.22)
(http://cvs.pgfoundry.org/cgi-bin/cvsweb.cgi/pg-migrator/pg_migrator/README?r1=1.21&r2=1.22)
^ permalink raw reply [nested|flat] 13+ messages in thread
* pgsql: Fix type-checking of RECORD-returning functions in FROM.
@ 2024-03-06 19:41 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 13+ messages in thread
From: Tom Lane @ 2024-03-06 19:41 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix type-checking of RECORD-returning functions in FROM.
In the corner case where a function returning RECORD has been
simplified to a RECORD constant or an inlined ROW() expression,
ExecInitFunctionScan failed to cross-check the function's result
rowtype against the coldeflist provided by the calling query.
That happened because get_expr_result_type is able to extract a
tupdesc from such expressions, which led ExecInitFunctionScan to
ignore the coldeflist. (Instead, it used the extracted tupdesc
to check the function's output, which of course always succeeds.)
I have not been able to demonstrate any really serious consequences
from this, because if some column of the result is of the wrong
type and is directly referenced by a Var of the calling query,
CheckVarSlotCompatibility will catch it. However, we definitely do
fail to report the case where the function returns more columns than
the coldeflist expects, and in the converse case where it returns
fewer columns, we get an assert failure (but, seemingly, no worse
results in non-assert builds).
To fix, always build the expected tupdesc from the coldeflist if there
is one, and consult get_expr_result_type only when there isn't one.
Also remove the failing Assert, even though it is no longer reached
after this fix. It doesn't seem to be adding anything useful, since
later checking will deal with cases with the wrong number of columns.
The only other place I could find that is doing something similar
is inline_set_returning_function. There's no live bug there because
we cannot be looking at a Const or RowExpr, but for consistency
change that code to agree with ExecInitFunctionScan.
Per report from PetSerAl. After some debate I've concluded that
this should be back-patched. There is a small risk that somebody
has been relying on such a case not throwing an error, but I judge
this outweighed by the risk that I've missed some way in which the
failure to cross-check has worse consequences than sketched above.
Discussion: https://postgr.es/m/CAKygsHSerA1eXsJHR9wft3Gn3wfHQ5RfP8XHBzF70_qcrrRvEg@mail.gmail.com
Branch
------
REL_15_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/3b671dcf53d1e1f5a08f8a4cd204207a8c1a065e
Modified Files
--------------
src/backend/executor/nodeFunctionscan.c | 81 ++++++++++++++++++--------------
src/backend/optimizer/util/clauses.c | 14 ++++--
src/test/regress/expected/rangefuncs.out | 13 +++++
src/test/regress/sql/rangefuncs.sql | 9 ++++
4 files changed, 76 insertions(+), 41 deletions(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
* pgsql: Fix type-checking of RECORD-returning functions in FROM.
@ 2024-03-06 19:41 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 13+ messages in thread
From: Tom Lane @ 2024-03-06 19:41 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix type-checking of RECORD-returning functions in FROM.
In the corner case where a function returning RECORD has been
simplified to a RECORD constant or an inlined ROW() expression,
ExecInitFunctionScan failed to cross-check the function's result
rowtype against the coldeflist provided by the calling query.
That happened because get_expr_result_type is able to extract a
tupdesc from such expressions, which led ExecInitFunctionScan to
ignore the coldeflist. (Instead, it used the extracted tupdesc
to check the function's output, which of course always succeeds.)
I have not been able to demonstrate any really serious consequences
from this, because if some column of the result is of the wrong
type and is directly referenced by a Var of the calling query,
CheckVarSlotCompatibility will catch it. However, we definitely do
fail to report the case where the function returns more columns than
the coldeflist expects, and in the converse case where it returns
fewer columns, we get an assert failure (but, seemingly, no worse
results in non-assert builds).
To fix, always build the expected tupdesc from the coldeflist if there
is one, and consult get_expr_result_type only when there isn't one.
Also remove the failing Assert, even though it is no longer reached
after this fix. It doesn't seem to be adding anything useful, since
later checking will deal with cases with the wrong number of columns.
The only other place I could find that is doing something similar
is inline_set_returning_function. There's no live bug there because
we cannot be looking at a Const or RowExpr, but for consistency
change that code to agree with ExecInitFunctionScan.
Per report from PetSerAl. After some debate I've concluded that
this should be back-patched. There is a small risk that somebody
has been relying on such a case not throwing an error, but I judge
this outweighed by the risk that I've missed some way in which the
failure to cross-check has worse consequences than sketched above.
Discussion: https://postgr.es/m/CAKygsHSerA1eXsJHR9wft3Gn3wfHQ5RfP8XHBzF70_qcrrRvEg@mail.gmail.com
Branch
------
REL_12_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/466376c9f848645a7391e9f1b19faca5bcab1360
Modified Files
--------------
src/backend/executor/nodeFunctionscan.c | 81 ++++++++++++++++++--------------
src/test/regress/expected/rangefuncs.out | 13 +++++
src/test/regress/sql/rangefuncs.sql | 9 ++++
3 files changed, 67 insertions(+), 36 deletions(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
* pgsql: Fix type-checking of RECORD-returning functions in FROM.
@ 2024-03-06 19:41 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 13+ messages in thread
From: Tom Lane @ 2024-03-06 19:41 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix type-checking of RECORD-returning functions in FROM.
In the corner case where a function returning RECORD has been
simplified to a RECORD constant or an inlined ROW() expression,
ExecInitFunctionScan failed to cross-check the function's result
rowtype against the coldeflist provided by the calling query.
That happened because get_expr_result_type is able to extract a
tupdesc from such expressions, which led ExecInitFunctionScan to
ignore the coldeflist. (Instead, it used the extracted tupdesc
to check the function's output, which of course always succeeds.)
I have not been able to demonstrate any really serious consequences
from this, because if some column of the result is of the wrong
type and is directly referenced by a Var of the calling query,
CheckVarSlotCompatibility will catch it. However, we definitely do
fail to report the case where the function returns more columns than
the coldeflist expects, and in the converse case where it returns
fewer columns, we get an assert failure (but, seemingly, no worse
results in non-assert builds).
To fix, always build the expected tupdesc from the coldeflist if there
is one, and consult get_expr_result_type only when there isn't one.
Also remove the failing Assert, even though it is no longer reached
after this fix. It doesn't seem to be adding anything useful, since
later checking will deal with cases with the wrong number of columns.
The only other place I could find that is doing something similar
is inline_set_returning_function. There's no live bug there because
we cannot be looking at a Const or RowExpr, but for consistency
change that code to agree with ExecInitFunctionScan.
Per report from PetSerAl. After some debate I've concluded that
this should be back-patched. There is a small risk that somebody
has been relying on such a case not throwing an error, but I judge
this outweighed by the risk that I've missed some way in which the
failure to cross-check has worse consequences than sketched above.
Discussion: https://postgr.es/m/CAKygsHSerA1eXsJHR9wft3Gn3wfHQ5RfP8XHBzF70_qcrrRvEg@mail.gmail.com
Branch
------
REL_14_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/a595c3075fb4b8ab4211b074485eb0a17062ec7f
Modified Files
--------------
src/backend/executor/nodeFunctionscan.c | 81 ++++++++++++++++++--------------
src/backend/optimizer/util/clauses.c | 14 ++++--
src/test/regress/expected/rangefuncs.out | 13 +++++
src/test/regress/sql/rangefuncs.sql | 9 ++++
4 files changed, 76 insertions(+), 41 deletions(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
* pgsql: Fix type-checking of RECORD-returning functions in FROM.
@ 2024-03-06 19:41 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 13+ messages in thread
From: Tom Lane @ 2024-03-06 19:41 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix type-checking of RECORD-returning functions in FROM.
In the corner case where a function returning RECORD has been
simplified to a RECORD constant or an inlined ROW() expression,
ExecInitFunctionScan failed to cross-check the function's result
rowtype against the coldeflist provided by the calling query.
That happened because get_expr_result_type is able to extract a
tupdesc from such expressions, which led ExecInitFunctionScan to
ignore the coldeflist. (Instead, it used the extracted tupdesc
to check the function's output, which of course always succeeds.)
I have not been able to demonstrate any really serious consequences
from this, because if some column of the result is of the wrong
type and is directly referenced by a Var of the calling query,
CheckVarSlotCompatibility will catch it. However, we definitely do
fail to report the case where the function returns more columns than
the coldeflist expects, and in the converse case where it returns
fewer columns, we get an assert failure (but, seemingly, no worse
results in non-assert builds).
To fix, always build the expected tupdesc from the coldeflist if there
is one, and consult get_expr_result_type only when there isn't one.
Also remove the failing Assert, even though it is no longer reached
after this fix. It doesn't seem to be adding anything useful, since
later checking will deal with cases with the wrong number of columns.
The only other place I could find that is doing something similar
is inline_set_returning_function. There's no live bug there because
we cannot be looking at a Const or RowExpr, but for consistency
change that code to agree with ExecInitFunctionScan.
Per report from PetSerAl. After some debate I've concluded that
this should be back-patched. There is a small risk that somebody
has been relying on such a case not throwing an error, but I judge
this outweighed by the risk that I've missed some way in which the
failure to cross-check has worse consequences than sketched above.
Discussion: https://postgr.es/m/CAKygsHSerA1eXsJHR9wft3Gn3wfHQ5RfP8XHBzF70_qcrrRvEg@mail.gmail.com
Branch
------
REL_13_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/d769f9d97feb4cd4d3bf485cd3fa80916d1e484f
Modified Files
--------------
src/backend/executor/nodeFunctionscan.c | 81 ++++++++++++++++++--------------
src/backend/optimizer/util/clauses.c | 14 ++++--
src/test/regress/expected/rangefuncs.out | 13 +++++
src/test/regress/sql/rangefuncs.sql | 9 ++++
4 files changed, 76 insertions(+), 41 deletions(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
* pgsql: Fix type-checking of RECORD-returning functions in FROM.
@ 2024-03-06 19:41 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 13+ messages in thread
From: Tom Lane @ 2024-03-06 19:41 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix type-checking of RECORD-returning functions in FROM.
In the corner case where a function returning RECORD has been
simplified to a RECORD constant or an inlined ROW() expression,
ExecInitFunctionScan failed to cross-check the function's result
rowtype against the coldeflist provided by the calling query.
That happened because get_expr_result_type is able to extract a
tupdesc from such expressions, which led ExecInitFunctionScan to
ignore the coldeflist. (Instead, it used the extracted tupdesc
to check the function's output, which of course always succeeds.)
I have not been able to demonstrate any really serious consequences
from this, because if some column of the result is of the wrong
type and is directly referenced by a Var of the calling query,
CheckVarSlotCompatibility will catch it. However, we definitely do
fail to report the case where the function returns more columns than
the coldeflist expects, and in the converse case where it returns
fewer columns, we get an assert failure (but, seemingly, no worse
results in non-assert builds).
To fix, always build the expected tupdesc from the coldeflist if there
is one, and consult get_expr_result_type only when there isn't one.
Also remove the failing Assert, even though it is no longer reached
after this fix. It doesn't seem to be adding anything useful, since
later checking will deal with cases with the wrong number of columns.
The only other place I could find that is doing something similar
is inline_set_returning_function. There's no live bug there because
we cannot be looking at a Const or RowExpr, but for consistency
change that code to agree with ExecInitFunctionScan.
Per report from PetSerAl. After some debate I've concluded that
this should be back-patched. There is a small risk that somebody
has been relying on such a case not throwing an error, but I judge
this outweighed by the risk that I've missed some way in which the
failure to cross-check has worse consequences than sketched above.
Discussion: https://postgr.es/m/CAKygsHSerA1eXsJHR9wft3Gn3wfHQ5RfP8XHBzF70_qcrrRvEg@mail.gmail.com
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/2ed8f9a01e74bfc63a56b2108f44237bc5eab9f5
Modified Files
--------------
src/backend/executor/nodeFunctionscan.c | 81 ++++++++++++++++++--------------
src/backend/optimizer/util/clauses.c | 14 ++++--
src/test/regress/expected/rangefuncs.out | 13 +++++
src/test/regress/sql/rangefuncs.sql | 9 ++++
4 files changed, 76 insertions(+), 41 deletions(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
* pgsql: Fix type-checking of RECORD-returning functions in FROM.
@ 2024-03-06 19:41 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 13+ messages in thread
From: Tom Lane @ 2024-03-06 19:41 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix type-checking of RECORD-returning functions in FROM.
In the corner case where a function returning RECORD has been
simplified to a RECORD constant or an inlined ROW() expression,
ExecInitFunctionScan failed to cross-check the function's result
rowtype against the coldeflist provided by the calling query.
That happened because get_expr_result_type is able to extract a
tupdesc from such expressions, which led ExecInitFunctionScan to
ignore the coldeflist. (Instead, it used the extracted tupdesc
to check the function's output, which of course always succeeds.)
I have not been able to demonstrate any really serious consequences
from this, because if some column of the result is of the wrong
type and is directly referenced by a Var of the calling query,
CheckVarSlotCompatibility will catch it. However, we definitely do
fail to report the case where the function returns more columns than
the coldeflist expects, and in the converse case where it returns
fewer columns, we get an assert failure (but, seemingly, no worse
results in non-assert builds).
To fix, always build the expected tupdesc from the coldeflist if there
is one, and consult get_expr_result_type only when there isn't one.
Also remove the failing Assert, even though it is no longer reached
after this fix. It doesn't seem to be adding anything useful, since
later checking will deal with cases with the wrong number of columns.
The only other place I could find that is doing something similar
is inline_set_returning_function. There's no live bug there because
we cannot be looking at a Const or RowExpr, but for consistency
change that code to agree with ExecInitFunctionScan.
Per report from PetSerAl. After some debate I've concluded that
this should be back-patched. There is a small risk that somebody
has been relying on such a case not throwing an error, but I judge
this outweighed by the risk that I've missed some way in which the
failure to cross-check has worse consequences than sketched above.
Discussion: https://postgr.es/m/CAKygsHSerA1eXsJHR9wft3Gn3wfHQ5RfP8XHBzF70_qcrrRvEg@mail.gmail.com
Branch
------
REL_16_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/1b3029be5df0b0c9abd72fad13283d6d5f4c6a9f
Modified Files
--------------
src/backend/executor/nodeFunctionscan.c | 81 ++++++++++++++++++--------------
src/backend/optimizer/util/clauses.c | 14 ++++--
src/test/regress/expected/rangefuncs.out | 13 +++++
src/test/regress/sql/rangefuncs.sql | 9 ++++
4 files changed, 76 insertions(+), 41 deletions(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
* pgsql: Fix type-checking of RECORD-returning functions in FROM, redux.
@ 2024-04-15 16:57 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 13+ messages in thread
From: Tom Lane @ 2024-04-15 16:57 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix type-checking of RECORD-returning functions in FROM, redux.
Commit 2ed8f9a01 intended to institute a policy that if a
RangeTblFunction has a coldeflist, then the function return type is
certainly RECORD, and we should use the coldeflist as the source of
truth about what the columns of the record type are. When the
original function has been folded to a constant, inspection of the
constant might give a different answer. This situation will lead to
a tuple-type-mismatch error at execution, but up until that point we
need to consistently believe the coldeflist, or we'll have problems
from different bits of code reaching different conclusions.
expandRTE didn't get that memo though, and would try to produce a
tupdesc based on the constant in this situation, leading to an
assertion failure. (Desultory testing suggests that non-assert
builds often manage to give the expected error, although I also
saw a "cache lookup failed for type 0" error, and it seems at
least possible that a crash could happen.)
Some other callers of get_expr_result_type and get_expr_result_tupdesc
were also being incautious about this. While none of them seem to
have actual bugs, they're working harder than necessary in this case,
besides which it seems safest to have an explicit policy of not using
those functions on an RTE with a coldeflist. Adjust the code
accordingly, and add commentary to funcapi.c about this policy.
Also fix an obsolete comment that claimed "get_expr_result_type()
doesn't know how to extract type info from a RECORD constant".
That hasn't been true since commit d57534740.
Per bug #18422 from Alexander Lakhin.
As with the previous commit, back-patch to all supported branches.
Discussion: https://postgr.es/m/18422-89ca86c8eac5246d@postgresql.org
Branch
------
REL_14_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/78e81e14db797713b26ee6fccb371128afa66b4e
Modified Files
--------------
src/backend/optimizer/prep/prepjointree.c | 4 ++++
src/backend/optimizer/util/clauses.c | 11 +++++------
src/backend/parser/parse_relation.c | 19 ++++++++++++++-----
src/backend/utils/fmgr/funcapi.c | 10 +++++++++-
src/test/regress/expected/rangefuncs.out | 3 +++
src/test/regress/sql/rangefuncs.sql | 1 +
6 files changed, 36 insertions(+), 12 deletions(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
* pgsql: Fix type-checking of RECORD-returning functions in FROM, redux.
@ 2024-04-15 16:57 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 13+ messages in thread
From: Tom Lane @ 2024-04-15 16:57 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix type-checking of RECORD-returning functions in FROM, redux.
Commit 2ed8f9a01 intended to institute a policy that if a
RangeTblFunction has a coldeflist, then the function return type is
certainly RECORD, and we should use the coldeflist as the source of
truth about what the columns of the record type are. When the
original function has been folded to a constant, inspection of the
constant might give a different answer. This situation will lead to
a tuple-type-mismatch error at execution, but up until that point we
need to consistently believe the coldeflist, or we'll have problems
from different bits of code reaching different conclusions.
expandRTE didn't get that memo though, and would try to produce a
tupdesc based on the constant in this situation, leading to an
assertion failure. (Desultory testing suggests that non-assert
builds often manage to give the expected error, although I also
saw a "cache lookup failed for type 0" error, and it seems at
least possible that a crash could happen.)
Some other callers of get_expr_result_type and get_expr_result_tupdesc
were also being incautious about this. While none of them seem to
have actual bugs, they're working harder than necessary in this case,
besides which it seems safest to have an explicit policy of not using
those functions on an RTE with a coldeflist. Adjust the code
accordingly, and add commentary to funcapi.c about this policy.
Also fix an obsolete comment that claimed "get_expr_result_type()
doesn't know how to extract type info from a RECORD constant".
That hasn't been true since commit d57534740.
Per bug #18422 from Alexander Lakhin.
As with the previous commit, back-patch to all supported branches.
Discussion: https://postgr.es/m/18422-89ca86c8eac5246d@postgresql.org
Branch
------
REL_16_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/cc1eb6a3cccdf656df29bf5dd585a4a7068fb814
Modified Files
--------------
src/backend/catalog/dependency.c | 6 +++++-
src/backend/optimizer/prep/prepjointree.c | 4 ++++
src/backend/optimizer/util/clauses.c | 11 +++++------
src/backend/parser/parse_relation.c | 19 ++++++++++++++-----
src/backend/utils/fmgr/funcapi.c | 10 +++++++++-
src/test/regress/expected/rangefuncs.out | 3 +++
src/test/regress/sql/rangefuncs.sql | 1 +
7 files changed, 41 insertions(+), 13 deletions(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
* pgsql: Fix type-checking of RECORD-returning functions in FROM, redux.
@ 2024-04-15 16:57 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 13+ messages in thread
From: Tom Lane @ 2024-04-15 16:57 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix type-checking of RECORD-returning functions in FROM, redux.
Commit 2ed8f9a01 intended to institute a policy that if a
RangeTblFunction has a coldeflist, then the function return type is
certainly RECORD, and we should use the coldeflist as the source of
truth about what the columns of the record type are. When the
original function has been folded to a constant, inspection of the
constant might give a different answer. This situation will lead to
a tuple-type-mismatch error at execution, but up until that point we
need to consistently believe the coldeflist, or we'll have problems
from different bits of code reaching different conclusions.
expandRTE didn't get that memo though, and would try to produce a
tupdesc based on the constant in this situation, leading to an
assertion failure. (Desultory testing suggests that non-assert
builds often manage to give the expected error, although I also
saw a "cache lookup failed for type 0" error, and it seems at
least possible that a crash could happen.)
Some other callers of get_expr_result_type and get_expr_result_tupdesc
were also being incautious about this. While none of them seem to
have actual bugs, they're working harder than necessary in this case,
besides which it seems safest to have an explicit policy of not using
those functions on an RTE with a coldeflist. Adjust the code
accordingly, and add commentary to funcapi.c about this policy.
Also fix an obsolete comment that claimed "get_expr_result_type()
doesn't know how to extract type info from a RECORD constant".
That hasn't been true since commit d57534740.
Per bug #18422 from Alexander Lakhin.
As with the previous commit, back-patch to all supported branches.
Discussion: https://postgr.es/m/18422-89ca86c8eac5246d@postgresql.org
Branch
------
REL_12_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/e0970862e8685e464a05135cfe813b27c686abb6
Modified Files
--------------
src/backend/optimizer/util/clauses.c | 11 +++++------
src/backend/parser/parse_relation.c | 19 ++++++++++++++-----
src/backend/utils/fmgr/funcapi.c | 10 +++++++++-
src/test/regress/expected/rangefuncs.out | 3 +++
src/test/regress/sql/rangefuncs.sql | 1 +
5 files changed, 32 insertions(+), 12 deletions(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
* pgsql: Fix type-checking of RECORD-returning functions in FROM, redux.
@ 2024-04-15 16:57 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 13+ messages in thread
From: Tom Lane @ 2024-04-15 16:57 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix type-checking of RECORD-returning functions in FROM, redux.
Commit 2ed8f9a01 intended to institute a policy that if a
RangeTblFunction has a coldeflist, then the function return type is
certainly RECORD, and we should use the coldeflist as the source of
truth about what the columns of the record type are. When the
original function has been folded to a constant, inspection of the
constant might give a different answer. This situation will lead to
a tuple-type-mismatch error at execution, but up until that point we
need to consistently believe the coldeflist, or we'll have problems
from different bits of code reaching different conclusions.
expandRTE didn't get that memo though, and would try to produce a
tupdesc based on the constant in this situation, leading to an
assertion failure. (Desultory testing suggests that non-assert
builds often manage to give the expected error, although I also
saw a "cache lookup failed for type 0" error, and it seems at
least possible that a crash could happen.)
Some other callers of get_expr_result_type and get_expr_result_tupdesc
were also being incautious about this. While none of them seem to
have actual bugs, they're working harder than necessary in this case,
besides which it seems safest to have an explicit policy of not using
those functions on an RTE with a coldeflist. Adjust the code
accordingly, and add commentary to funcapi.c about this policy.
Also fix an obsolete comment that claimed "get_expr_result_type()
doesn't know how to extract type info from a RECORD constant".
That hasn't been true since commit d57534740.
Per bug #18422 from Alexander Lakhin.
As with the previous commit, back-patch to all supported branches.
Discussion: https://postgr.es/m/18422-89ca86c8eac5246d@postgresql.org
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/e0df80828abc0c01fcde123389032370b98f2015
Modified Files
--------------
src/backend/catalog/dependency.c | 6 +++++-
src/backend/optimizer/prep/prepjointree.c | 4 ++++
src/backend/optimizer/util/clauses.c | 11 +++++------
src/backend/parser/parse_relation.c | 19 ++++++++++++++-----
src/backend/utils/fmgr/funcapi.c | 10 +++++++++-
src/test/regress/expected/rangefuncs.out | 3 +++
src/test/regress/sql/rangefuncs.sql | 1 +
7 files changed, 41 insertions(+), 13 deletions(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
* pgsql: Fix type-checking of RECORD-returning functions in FROM, redux.
@ 2024-04-15 16:57 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 13+ messages in thread
From: Tom Lane @ 2024-04-15 16:57 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix type-checking of RECORD-returning functions in FROM, redux.
Commit 2ed8f9a01 intended to institute a policy that if a
RangeTblFunction has a coldeflist, then the function return type is
certainly RECORD, and we should use the coldeflist as the source of
truth about what the columns of the record type are. When the
original function has been folded to a constant, inspection of the
constant might give a different answer. This situation will lead to
a tuple-type-mismatch error at execution, but up until that point we
need to consistently believe the coldeflist, or we'll have problems
from different bits of code reaching different conclusions.
expandRTE didn't get that memo though, and would try to produce a
tupdesc based on the constant in this situation, leading to an
assertion failure. (Desultory testing suggests that non-assert
builds often manage to give the expected error, although I also
saw a "cache lookup failed for type 0" error, and it seems at
least possible that a crash could happen.)
Some other callers of get_expr_result_type and get_expr_result_tupdesc
were also being incautious about this. While none of them seem to
have actual bugs, they're working harder than necessary in this case,
besides which it seems safest to have an explicit policy of not using
those functions on an RTE with a coldeflist. Adjust the code
accordingly, and add commentary to funcapi.c about this policy.
Also fix an obsolete comment that claimed "get_expr_result_type()
doesn't know how to extract type info from a RECORD constant".
That hasn't been true since commit d57534740.
Per bug #18422 from Alexander Lakhin.
As with the previous commit, back-patch to all supported branches.
Discussion: https://postgr.es/m/18422-89ca86c8eac5246d@postgresql.org
Branch
------
REL_13_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/b6e21cef72f1f93b0ec4d6da4d9c2e8bb3fca98a
Modified Files
--------------
src/backend/optimizer/prep/prepjointree.c | 4 ++++
src/backend/optimizer/util/clauses.c | 11 +++++------
src/backend/parser/parse_relation.c | 19 ++++++++++++++-----
src/backend/utils/fmgr/funcapi.c | 10 +++++++++-
src/test/regress/expected/rangefuncs.out | 3 +++
src/test/regress/sql/rangefuncs.sql | 1 +
6 files changed, 36 insertions(+), 12 deletions(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
* pgsql: Fix type-checking of RECORD-returning functions in FROM, redux.
@ 2024-04-15 16:57 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 13+ messages in thread
From: Tom Lane @ 2024-04-15 16:57 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix type-checking of RECORD-returning functions in FROM, redux.
Commit 2ed8f9a01 intended to institute a policy that if a
RangeTblFunction has a coldeflist, then the function return type is
certainly RECORD, and we should use the coldeflist as the source of
truth about what the columns of the record type are. When the
original function has been folded to a constant, inspection of the
constant might give a different answer. This situation will lead to
a tuple-type-mismatch error at execution, but up until that point we
need to consistently believe the coldeflist, or we'll have problems
from different bits of code reaching different conclusions.
expandRTE didn't get that memo though, and would try to produce a
tupdesc based on the constant in this situation, leading to an
assertion failure. (Desultory testing suggests that non-assert
builds often manage to give the expected error, although I also
saw a "cache lookup failed for type 0" error, and it seems at
least possible that a crash could happen.)
Some other callers of get_expr_result_type and get_expr_result_tupdesc
were also being incautious about this. While none of them seem to
have actual bugs, they're working harder than necessary in this case,
besides which it seems safest to have an explicit policy of not using
those functions on an RTE with a coldeflist. Adjust the code
accordingly, and add commentary to funcapi.c about this policy.
Also fix an obsolete comment that claimed "get_expr_result_type()
doesn't know how to extract type info from a RECORD constant".
That hasn't been true since commit d57534740.
Per bug #18422 from Alexander Lakhin.
As with the previous commit, back-patch to all supported branches.
Discussion: https://postgr.es/m/18422-89ca86c8eac5246d@postgresql.org
Branch
------
REL_15_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/09989ba84734c07d045ec379c9424508a52ac604
Modified Files
--------------
src/backend/catalog/dependency.c | 6 +++++-
src/backend/optimizer/prep/prepjointree.c | 4 ++++
src/backend/optimizer/util/clauses.c | 11 +++++------
src/backend/parser/parse_relation.c | 19 ++++++++++++++-----
src/backend/utils/fmgr/funcapi.c | 10 +++++++++-
src/test/regress/expected/rangefuncs.out | 3 +++
src/test/regress/sql/rangefuncs.sql | 1 +
7 files changed, 41 insertions(+), 13 deletions(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2024-04-15 16:57 UTC | newest]
Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2009-06-01 21:47 pg-migrator - pg_migrator: Document limitations. User Bmomjian <bmomjian@pgfoundry.org>
2024-03-06 19:41 pgsql: Fix type-checking of RECORD-returning functions in FROM. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-06 19:41 pgsql: Fix type-checking of RECORD-returning functions in FROM. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-06 19:41 pgsql: Fix type-checking of RECORD-returning functions in FROM. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-06 19:41 pgsql: Fix type-checking of RECORD-returning functions in FROM. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-06 19:41 pgsql: Fix type-checking of RECORD-returning functions in FROM. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-06 19:41 pgsql: Fix type-checking of RECORD-returning functions in FROM. Tom Lane <tgl@sss.pgh.pa.us>
2024-04-15 16:57 pgsql: Fix type-checking of RECORD-returning functions in FROM, redux. Tom Lane <tgl@sss.pgh.pa.us>
2024-04-15 16:57 pgsql: Fix type-checking of RECORD-returning functions in FROM, redux. Tom Lane <tgl@sss.pgh.pa.us>
2024-04-15 16:57 pgsql: Fix type-checking of RECORD-returning functions in FROM, redux. Tom Lane <tgl@sss.pgh.pa.us>
2024-04-15 16:57 pgsql: Fix type-checking of RECORD-returning functions in FROM, redux. Tom Lane <tgl@sss.pgh.pa.us>
2024-04-15 16:57 pgsql: Fix type-checking of RECORD-returning functions in FROM, redux. Tom Lane <tgl@sss.pgh.pa.us>
2024-04-15 16:57 pgsql: Fix type-checking of RECORD-returning functions in FROM, redux. Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox