Re: pgsql: Fix page modification outside of critical section in GIN
On Sun, Feb 9, 2020 at 8:05 PM Tom Lane wrote: > > I wrote: > > Alexander Korotkov writes: > >> Fix page modification outside of critical section in GIN > > > I happened to notice, while reviewing stuff for the release notes, > > that this patch does not do what the commit message says. The > > previous code modified the pd_prune_xid of the "dBuffer" page, > > but now it's modifying the pd_prune_xid of the "lBuffer" page. > > Is that actually correct? > > Since we're rapidly approaching the wrap deadline for this week's > releases, I took it upon myself to review this code more carefully, > and concluded that indeed it isn't correct. I pushed a fix that > moves the GinPageSetDeleteXid call again. Sorry for delayed response. Yes, my fix for oversight contain oversight itself. I confirm the fix you've committed is correct. Thank you! -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
pgsql: Avoid full scan of GIN indexes when possible
Avoid full scan of GIN indexes when possible The strategy of GIN index scan is driven by opclass-specific extract_query method. This method that needed search mode is GIN_SEARCH_MODE_ALL. This mode means that matching tuple may contain none of extracted entries. Simple example is '!term' tsquery, which doesn't need any term to exist in matching tsvector. In order to handle such scan key GIN calculates virtual entry, which contains all TIDs of all entries of attribute. In fact this is full scan of index attribute. And typically this is very slow, but allows to handle some queries correctly in GIN. However, current algorithm calculate such virtual entry for each GIN_SEARCH_MODE_ALL scan key even if they are multiple for the same attribute. This is clearly not optimal. This commit improves the situation by introduction of "exclude only" scan keys. Such scan keys are not capable to return set of matching TIDs. Instead, they are capable only to filter TIDs produced by normal scan keys. Therefore, each attribute should contain at least one normal scan key, while rest of them may be "exclude only" if search mode is GIN_SEARCH_MODE_ALL. The same optimization might be applied to the whole scan, not per-attribute. But that leads to NULL values elimination problem. There is trade-off between multiple possible ways to do this. We probably want to do this later using some cost-based decision algorithm. Discussion: https://postgr.es/m/CAOBaU_YGP5-BEt5Cc0%3DzMve92vocPzD%2BXiZgiZs1kjY0cj%3DXBg%40mail.gmail.com Author: Nikita Glukhov, Alexander Korotkov, Tom Lane, Julien Rouhaud Reviewed-by: Julien Rouhaud, Tomas Vondra, Tom Lane Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/4b754d6c16e16cc1a1adf12ab0f48603069a0efd Modified Files -- contrib/pg_trgm/expected/pg_trgm.out | 101 ++ contrib/pg_trgm/sql/pg_trgm.sql | 27 +++ src/backend/access/gin/ginget.c | 84 -- src/backend/access/gin/ginscan.c | 127 +++- src/backend/utils/adt/selfuncs.c | 37 -- src/include/access/gin_private.h | 14 src/test/regress/expected/gin.out | 131 +- src/test/regress/expected/tsearch.out | 35 + src/test/regress/sql/gin.sql | 91 ++- src/test/regress/sql/tsearch.sql | 9 +++ 10 files changed, 579 insertions(+), 77 deletions(-)
pgsql: Fix page modification outside of critical section in GIN
Fix page modification outside of critical section in GIN By oversight 52ac6cd2d0 makes ginDeletePage() sets pd_prune_xid of page to be deleted before entering the critical section. It appears that only versions 11 and later were affected by this oversight. Backpatch-through: 11 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/b107140804817cc30a4069b1bb5545aa3ea0ce6c Modified Files -- src/backend/access/gin/ginvacuum.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
pgsql: Fix page modification outside of critical section in GIN
Fix page modification outside of critical section in GIN By oversight 52ac6cd2d0 makes ginDeletePage() sets pd_prune_xid of page to be deleted before entering the critical section. It appears that only versions 11 and later were affected by this oversight. Backpatch-through: 11 Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/7d467dee0831728de107ad15c5625bf97698d790 Modified Files -- src/backend/access/gin/ginvacuum.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
pgsql: Fix page modification outside of critical section in GIN
Fix page modification outside of critical section in GIN By oversight 52ac6cd2d0 makes ginDeletePage() sets pd_prune_xid of page to be deleted before entering the critical section. It appears that only versions 11 and later were affected by this oversight. Backpatch-through: 11 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/a64e7e05a418ec26a76ecbf04c80e9ba7fe8aa90 Modified Files -- src/backend/access/gin/ginvacuum.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
pgsql: Revise GIN README
Revise GIN README We find GIN concurrency bugs from time to time. One of the problems here is that concurrency of GIN isn't well-documented in README. So, it might be even hard to distinguish design bugs from implementation bugs. This commit revised concurrency section in GIN README providing more details. Some examples are illustrated in ASCII art. Also, this commit add the explanation of how is tuple layout in internal GIN B-tree page different in comparison with nbtree. Discussion: https://postgr.es/m/CAPpHfduXR_ywyaVN4%2BOYEGaw%3DcPLzWX6RxYLBncKw8de9vOkqw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/8165384babd96258ed521c4ea2567c70a1849948 Modified Files -- src/backend/access/gin/README | 209 +- 1 file changed, 168 insertions(+), 41 deletions(-)
pgsql: Revise GIN README
Revise GIN README We find GIN concurrency bugs from time to time. One of the problems here is that concurrency of GIN isn't well-documented in README. So, it might be even hard to distinguish design bugs from implementation bugs. This commit revised concurrency section in GIN README providing more details. Some examples are illustrated in ASCII art. Also, this commit add the explanation of how is tuple layout in internal GIN B-tree page different in comparison with nbtree. Discussion: https://postgr.es/m/CAPpHfduXR_ywyaVN4%2BOYEGaw%3DcPLzWX6RxYLBncKw8de9vOkqw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- REL9_4_STABLE Details --- https://git.postgresql.org/pg/commitdiff/91ce01a6e07b27e8c293abe4d2d565e0d8ae4021 Modified Files -- src/backend/access/gin/README | 209 +- 1 file changed, 168 insertions(+), 41 deletions(-)
pgsql: Fix traversing to the deleted GIN page via downlink
Fix traversing to the deleted GIN page via downlink Current GIN code appears to don't handle traversing to the deleted page via downlink. This commit fixes that by stepping right from the delete page like we do in nbtree. This commit also fixes setting 'deleted' flag to the GIN pages. Now other page flags are not erased once page is deleted. That helps to keep our assertions true if we arrive deleted page via downlink. Discussion: https://postgr.es/m/CAPpHfdvMvsw-NcE5bRS7R1BbvA4BxoDnVVjkXC5W0Czvy9LVrg%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- REL9_4_STABLE Details --- https://git.postgresql.org/pg/commitdiff/1414821e1688c86266c5ecaad378086785152332 Modified Files -- src/backend/access/gin/ginbtree.c| 7 --- src/backend/access/gin/gindatapage.c | 3 +++ src/backend/access/gin/ginvacuum.c | 2 +- src/backend/access/gin/ginxlog.c | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-)
pgsql: Fix traversing to the deleted GIN page via downlink
Fix traversing to the deleted GIN page via downlink Current GIN code appears to don't handle traversing to the deleted page via downlink. This commit fixes that by stepping right from the delete page like we do in nbtree. This commit also fixes setting 'deleted' flag to the GIN pages. Now other page flags are not erased once page is deleted. That helps to keep our assertions true if we arrive deleted page via downlink. Discussion: https://postgr.es/m/CAPpHfdvMvsw-NcE5bRS7R1BbvA4BxoDnVVjkXC5W0Czvy9LVrg%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/4fc4856849deb51c8d8822d79b6f10d571dab4f6 Modified Files -- src/backend/access/gin/ginbtree.c| 7 --- src/backend/access/gin/gindatapage.c | 3 +++ src/backend/access/gin/ginvacuum.c | 2 +- src/backend/access/gin/ginxlog.c | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-)
pgsql: Fix traversing to the deleted GIN page via downlink
Fix traversing to the deleted GIN page via downlink Current GIN code appears to don't handle traversing to the deleted page via downlink. This commit fixes that by stepping right from the delete page like we do in nbtree. This commit also fixes setting 'deleted' flag to the GIN pages. Now other page flags are not erased once page is deleted. That helps to keep our assertions true if we arrive deleted page via downlink. Discussion: https://postgr.es/m/CAPpHfdvMvsw-NcE5bRS7R1BbvA4BxoDnVVjkXC5W0Czvy9LVrg%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/99f5888d358a5db375ce0299b18fb47ccfa1646c Modified Files -- src/backend/access/gin/ginbtree.c| 7 --- src/backend/access/gin/gindatapage.c | 3 +++ src/backend/access/gin/ginvacuum.c | 2 +- src/backend/access/gin/ginxlog.c | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-)
pgsql: Fix deadlock between ginDeletePage() and ginStepRight()
Fix deadlock between ginDeletePage() and ginStepRight() When ginDeletePage() is about to delete page it locks its left sibling to revise the rightlink. So, it locks pages in right to left manner. Int he same time ginStepRight() locks pages in left to right manner, and that could cause a deadlock. This commit makes ginScanToDelete() keep exclusive lock on left siblings of currently investigated path. That elimites need to relock left sibling in ginDeletePage(). Thus, deadlock with ginStepRight() can't happen anymore. Reported-by: Chen Huajun Discussion: https://postgr.es/m/5c332bd1.87b6.16d7c17aa98.Coremail.chjischj%40163.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 10 Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/21ad61ab31786128ae780feca8fcf766bbb6a579 Modified Files -- src/backend/access/gin/README | 79 ++ src/backend/access/gin/ginvacuum.c | 54 +- 2 files changed, 81 insertions(+), 52 deletions(-)
pgsql: Revise GIN README
Revise GIN README We find GIN concurrency bugs from time to time. One of the problems here is that concurrency of GIN isn't well-documented in README. So, it might be even hard to distinguish design bugs from implementation bugs. This commit revised concurrency section in GIN README providing more details. Some examples are illustrated in ASCII art. Also, this commit add the explanation of how is tuple layout in internal GIN B-tree page different in comparison with nbtree. Discussion: https://postgr.es/m/CAPpHfduXR_ywyaVN4%2BOYEGaw%3DcPLzWX6RxYLBncKw8de9vOkqw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/b6f57c2d7bfc2c0ddfc33158b2c42ebd0c715959 Modified Files -- src/backend/access/gin/README | 214 ++ 1 file changed, 176 insertions(+), 38 deletions(-)
pgsql: Fix traversing to the deleted GIN page via downlink
Fix traversing to the deleted GIN page via downlink Current GIN code appears to don't handle traversing to the deleted page via downlink. This commit fixes that by stepping right from the delete page like we do in nbtree. This commit also fixes setting 'deleted' flag to the GIN pages. Now other page flags are not erased once page is deleted. That helps to keep our assertions true if we arrive deleted page via downlink. Discussion: https://postgr.es/m/CAPpHfdvMvsw-NcE5bRS7R1BbvA4BxoDnVVjkXC5W0Czvy9LVrg%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/d5ad7a09afd066dce423f282bb2b338f48614d32 Modified Files -- src/backend/access/gin/ginbtree.c| 7 --- src/backend/access/gin/gindatapage.c | 3 +++ src/backend/access/gin/ginvacuum.c | 2 +- src/backend/access/gin/ginxlog.c | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-)
pgsql: Revise GIN README
Revise GIN README We find GIN concurrency bugs from time to time. One of the problems here is that concurrency of GIN isn't well-documented in README. So, it might be even hard to distinguish design bugs from implementation bugs. This commit revised concurrency section in GIN README providing more details. Some examples are illustrated in ASCII art. Also, this commit add the explanation of how is tuple layout in internal GIN B-tree page different in comparison with nbtree. Discussion: https://postgr.es/m/CAPpHfduXR_ywyaVN4%2BOYEGaw%3DcPLzWX6RxYLBncKw8de9vOkqw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/287192bda245cbb7b7577d4c351822772b7e7373 Modified Files -- src/backend/access/gin/README | 214 ++ 1 file changed, 176 insertions(+), 38 deletions(-)
pgsql: Revise GIN README
Revise GIN README We find GIN concurrency bugs from time to time. One of the problems here is that concurrency of GIN isn't well-documented in README. So, it might be even hard to distinguish design bugs from implementation bugs. This commit revised concurrency section in GIN README providing more details. Some examples are illustrated in ASCII art. Also, this commit add the explanation of how is tuple layout in internal GIN B-tree page different in comparison with nbtree. Discussion: https://postgr.es/m/CAPpHfduXR_ywyaVN4%2BOYEGaw%3DcPLzWX6RxYLBncKw8de9vOkqw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/84dcf5235984f45458d13a9e0e486caf97f152ea Modified Files -- src/backend/access/gin/README | 209 +- 1 file changed, 168 insertions(+), 41 deletions(-)
pgsql: Fix deadlock between ginDeletePage() and ginStepRight()
Fix deadlock between ginDeletePage() and ginStepRight() When ginDeletePage() is about to delete page it locks its left sibling to revise the rightlink. So, it locks pages in right to left manner. Int he same time ginStepRight() locks pages in left to right manner, and that could cause a deadlock. This commit makes ginScanToDelete() keep exclusive lock on left siblings of currently investigated path. That elimites need to relock left sibling in ginDeletePage(). Thus, deadlock with ginStepRight() can't happen anymore. Reported-by: Chen Huajun Discussion: https://postgr.es/m/5c332bd1.87b6.16d7c17aa98.Coremail.chjischj%40163.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 10 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/051c50c01c32ceb498d53e78f48297713744b7cb Modified Files -- src/backend/access/gin/README | 79 ++ src/backend/access/gin/ginvacuum.c | 54 +- 2 files changed, 81 insertions(+), 52 deletions(-)
pgsql: Fix traversing to the deleted GIN page via downlink
Fix traversing to the deleted GIN page via downlink Current GIN code appears to don't handle traversing to the deleted page via downlink. This commit fixes that by stepping right from the delete page like we do in nbtree. This commit also fixes setting 'deleted' flag to the GIN pages. Now other page flags are not erased once page is deleted. That helps to keep our assertions true if we arrive deleted page via downlink. Discussion: https://postgr.es/m/CAPpHfdvMvsw-NcE5bRS7R1BbvA4BxoDnVVjkXC5W0Czvy9LVrg%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/ab64b474d9bec8486b42e550526a121f0d81b681 Modified Files -- src/backend/access/gin/ginbtree.c| 7 --- src/backend/access/gin/gindatapage.c | 3 +++ src/backend/access/gin/ginvacuum.c | 2 +- src/backend/access/gin/ginxlog.c | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-)
pgsql: Revise GIN README
Revise GIN README We find GIN concurrency bugs from time to time. One of the problems here is that concurrency of GIN isn't well-documented in README. So, it might be even hard to distinguish design bugs from implementation bugs. This commit revised concurrency section in GIN README providing more details. Some examples are illustrated in ASCII art. Also, this commit add the explanation of how is tuple layout in internal GIN B-tree page different in comparison with nbtree. Discussion: https://postgr.es/m/CAPpHfduXR_ywyaVN4%2BOYEGaw%3DcPLzWX6RxYLBncKw8de9vOkqw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/32ca32d0bed4b95e5cd63998478a7816a89cd43d Modified Files -- src/backend/access/gin/README | 214 ++ 1 file changed, 176 insertions(+), 38 deletions(-)
pgsql: Fix deadlock between ginDeletePage() and ginStepRight()
Fix deadlock between ginDeletePage() and ginStepRight() When ginDeletePage() is about to delete page it locks its left sibling to revise the rightlink. So, it locks pages in right to left manner. Int he same time ginStepRight() locks pages in left to right manner, and that could cause a deadlock. This commit makes ginScanToDelete() keep exclusive lock on left siblings of currently investigated path. That elimites need to relock left sibling in ginDeletePage(). Thus, deadlock with ginStepRight() can't happen anymore. Reported-by: Chen Huajun Discussion: https://postgr.es/m/5c332bd1.87b6.16d7c17aa98.Coremail.chjischj%40163.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 10 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/e14641197a5690d92cc48446d0d40f1aec07bac7 Modified Files -- src/backend/access/gin/README | 79 ++ src/backend/access/gin/ginvacuum.c | 54 +- 2 files changed, 81 insertions(+), 52 deletions(-)
pgsql: Fix deadlock between ginDeletePage() and ginStepRight()
Fix deadlock between ginDeletePage() and ginStepRight() When ginDeletePage() is about to delete page it locks its left sibling to revise the rightlink. So, it locks pages in right to left manner. Int he same time ginStepRight() locks pages in left to right manner, and that could cause a deadlock. This commit makes ginScanToDelete() keep exclusive lock on left siblings of currently investigated path. That elimites need to relock left sibling in ginDeletePage(). Thus, deadlock with ginStepRight() can't happen anymore. Reported-by: Chen Huajun Discussion: https://postgr.es/m/5c332bd1.87b6.16d7c17aa98.Coremail.chjischj%40163.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 10 Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/9f292798992ee8efe3281ba61db3c53ea7007b1a Modified Files -- src/backend/access/gin/README | 79 ++ src/backend/access/gin/ginvacuum.c | 54 +- 2 files changed, 81 insertions(+), 52 deletions(-)
pgsql: Fix traversing to the deleted GIN page via downlink
Fix traversing to the deleted GIN page via downlink Current GIN code appears to don't handle traversing to the deleted page via downlink. This commit fixes that by stepping right from the delete page like we do in nbtree. This commit also fixes setting 'deleted' flag to the GIN pages. Now other page flags are not erased once page is deleted. That helps to keep our assertions true if we arrive deleted page via downlink. Discussion: https://postgr.es/m/CAPpHfdvMvsw-NcE5bRS7R1BbvA4BxoDnVVjkXC5W0Czvy9LVrg%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/c0bf35421529ab210479ea0e828270e6626adff6 Modified Files -- src/backend/access/gin/ginbtree.c| 7 --- src/backend/access/gin/gindatapage.c | 3 +++ src/backend/access/gin/ginvacuum.c | 2 +- src/backend/access/gin/ginxlog.c | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-)
pgsql: Revise GIN README
Revise GIN README We find GIN concurrency bugs from time to time. One of the problems here is that concurrency of GIN isn't well-documented in README. So, it might be even hard to distinguish design bugs from implementation bugs. This commit revised concurrency section in GIN README providing more details. Some examples are illustrated in ASCII art. Also, this commit add the explanation of how is tuple layout in internal GIN B-tree page different in comparison with nbtree. Discussion: https://postgr.es/m/CAPpHfduXR_ywyaVN4%2BOYEGaw%3DcPLzWX6RxYLBncKw8de9vOkqw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/ca05fa5375eb3124660e24346f1bdc3990c118f2 Modified Files -- src/backend/access/gin/README | 214 ++ 1 file changed, 176 insertions(+), 38 deletions(-)
pgsql: Fix traversing to the deleted GIN page via downlink
Fix traversing to the deleted GIN page via downlink Current GIN code appears to don't handle traversing to the deleted page via downlink. This commit fixes that by stepping right from the delete page like we do in nbtree. This commit also fixes setting 'deleted' flag to the GIN pages. Now other page flags are not erased once page is deleted. That helps to keep our assertions true if we arrive deleted page via downlink. Discussion: https://postgr.es/m/CAPpHfdvMvsw-NcE5bRS7R1BbvA4BxoDnVVjkXC5W0Czvy9LVrg%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Peter Geoghegan Backpatch-through: 9.4 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/ee437ca7408b67f2b785e4221c7e567aecf68b47 Modified Files -- src/backend/access/gin/ginbtree.c| 7 --- src/backend/access/gin/gindatapage.c | 3 +++ src/backend/access/gin/ginvacuum.c | 2 +- src/backend/access/gin/ginxlog.c | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-)
pgsql: Refactor timestamp2timestamptz_opt_error()
Refactor timestamp2timestamptz_opt_error() While casting from timestamp to timestamptz we do timestamp2tm() then tm2timestamp(). This commit eliminates call to tm2timestamp(). Instead, it directly applies timezone offset to the original timestamp value. That makes upcoming datetime overflow handling in jsonpath easier. That should also save us some CPU cycles. Discussion: https://postgr.es/m/CAPpHfdvRPRh_mTGar5WmDeRZ%3DU5dOXHdxspYYD%3D76m3knNGjXA%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Tom Lane Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/a6888fde7f0dbe865559b31ba2ce01ac1150debe Modified Files -- src/backend/utils/adt/timestamp.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-)
pgsql: Refactor jsonpath's compareDatetime()
Refactor jsonpath's compareDatetime() This commit refactors come ridiculous coding in compareDatetime(). Also, it provides correct cross-datatype comparison even when one of values overflows during cast. That eliminates dilemma on whether we should suppress overflow errors during cast. Reported-by: Tom Lane Discussion: https://postgr.es/m/32308.1569455803%40sss.pgh.pa.us Discussion: https://postgr.es/m/a5629d0c-8162-7559-16aa-0c8390d6ba5f%40postgrespro.ru Author: Nikita Glukhov, Alexander Korotkov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/52ad1e659967896ed153185328ffe806d69abcb6 Modified Files -- src/backend/utils/adt/date.c | 40 ++-- src/backend/utils/adt/jsonpath_exec.c| 262 +-- src/backend/utils/adt/timestamp.c| 31 ++-- src/include/utils/date.h | 4 +- src/include/utils/timestamp.h| 4 +- src/test/regress/expected/jsonb_jsonpath.out | 37 ++-- src/test/regress/sql/jsonb_jsonpath.sql | 3 + 7 files changed, 237 insertions(+), 144 deletions(-)
Re: pgsql: Implement jsonpath .datetime() method
On Mon, Oct 14, 2019 at 5:36 AM Alexander Korotkov wrote: > On Sun, Oct 13, 2019 at 5:24 AM Tom Lane wrote: > > Alexander Korotkov writes: > > > This patch also changes the way timestamp to timestamptz cast works. > > > Previously it did timestamp2tm() then tm2timestamp(). Instead, after > > > timestamp2tm() it calculates timezone offset and applies it to > > > original timestamp value. I hope this is correct. > > > > I'd wonder whether this gives the same answers near DST transitions, > > where it's not real clear which offset applies. > > I will try this and share the results. I've separated refactoring of timestamp to timestamptz cast into a separate patch. Patchset is attached. I've investigates the behavior near DST transitions in Moscow timezone. Last two DST transitions it had in 2010-03-28 and 2010-10-31. It behaves the same with and without patch. The tests are below. # set timezone = 'Europe/Moscow'; # select '2010-03-28 01:59:59'::timestamp::timestamptz; timestamptz 2010-03-28 01:59:59+03 (1 row) # select '2010-03-28 02:00:00'::timestamp::timestamptz; timestamptz 2010-03-28 03:00:00+04 (1 row) # select '2010-03-28 02:59:59'::timestamp::timestamptz; timestamptz 2010-03-28 03:59:59+04 (1 row) # select '2010-03-28 03:00:00'::timestamp::timestamptz; timestamptz 2010-03-28 03:00:00+04 (1 row) # select '2010-10-31 01:59:59'::timestamp::timestamptz; timestamptz 2010-10-31 01:59:59+04 (1 row) # select '2010-10-31 02:00:00'::timestamp::timestamptz; timestamptz 2010-10-31 02:00:00+03 (1 row) BTW, I've noticed how ridiculous cast behaves for values in the range of [2010-03-28 02:00:00, 2010-03-28 03:00:00). Now, I think that timestamptz type, which explicitly stores timezone offset, has some point. At least, it would be possible to save the same local time value during casts. I'm going to push these two patches if no objections. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company 0001-Refactor-timestamp2timestamptz_opt_error-3.patch Description: Binary data 0002-Refactor-jsonpath-s-compareDatetime-3.patch Description: Binary data
Re: pgsql: Implement jsonpath .datetime() method
On Sun, Oct 13, 2019 at 5:24 AM Tom Lane wrote: > Alexander Korotkov writes: > > This patch also changes the way timestamp to timestamptz cast works. > > Previously it did timestamp2tm() then tm2timestamp(). Instead, after > > timestamp2tm() it calculates timezone offset and applies it to > > original timestamp value. I hope this is correct. > > I'd wonder whether this gives the same answers near DST transitions, > where it's not real clear which offset applies. I will try this and share the results. > Please *don't* wrap this sort of thing into an unrelated feature patch. Sure, thank you for noticing. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: pgsql: Implement jsonpath .datetime() method
On Thu, Oct 3, 2019 at 4:48 PM Robert Haas wrote: > On Tue, Oct 1, 2019 at 1:41 PM Alexander Korotkov > wrote: > > So, basically standard requires us to suppress any error happening in > > filter expression. > > Sounds like the standard is dumb, then. :-) > > > But as I wrote before suppression of errors in > > datetime comparison may lead to surprising results. That happens in > > rare corner cases, but still. This makes uneasy choice between > > consistent behavior and standard behavior. > > Yeah. Proposed patch eliminates this dilemma in particular case. It provides correct cross-type comparison of datetime values even if one of values overflows during cast. In order to do this, I made cast functions to report whether lower or upper boundary is overflowed. We know that overflowed value is lower (or upper) than any valid value except infinity. This patch also changes the way timestamp to timestamptz cast works. Previously it did timestamp2tm() then tm2timestamp(). Instead, after timestamp2tm() it calculates timezone offset and applies it to original timestamp value. I hope this is correct. If so, besides making overflow handling easier, this refactoring saves some CPU cycles. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company 0002-Refactor-jsonpath-s-compareDatetime-2.patch Description: Binary data
Re: pgsql: Implement jsonpath .datetime() method
On Mon, Sep 30, 2019 at 10:56 PM Robert Haas wrote: > On Sun, Sep 29, 2019 at 10:30 AM Alexander Korotkov > wrote: > > So, jsonpath behaves like 100 is not greater than 2020. This > > looks like plain false. And user can't expect that unless she is > > familiar with our particular issues. Now I got opinion that such > > errors shouldn't be suppressed. We can't suppress *every* error. If > > trying to do this, we can come to an idea to suppress OOM error and > > return garbage then, which is obviously ridiculous. Opinions? > > I don't know enough about jsonpath to have a view on specifically > which errors ought to be suppressed, but I agree that it's probably > not all of them. In fact, I'd go so far as to say that thinking about > it in terms of error suppression is probably not the right approach in > the first place. Rather, you want to ask what behavior you're trying > to create. > > For example, if I'm trying to write a function that takes a string as > input and returns JSON, where the result is formatted as a number if > possible or a string otherwise, I might want access at the C level to > the guts of numeric_in, with all parsing errors returned rather than > thrown. But it would be silly to suppress an out-of-memory condition, > because that doesn't help the caller. The caller wants to know whether > the thing can be parsed as a number or not, and that has nothing to do > with whether we're out of memory, so an out-of-memory error should > still be thrown. > > In this case here, it seems to me that you should similarly start by > defining the behavior you're trying to create. Unless that's clearly > defined, deciding which errors to suppress may be difficult. Making C functions return errors rather than throw is what we're implementing in our patchsets. In big picture the behavior we're trying to create is SQL Standard 2016. It defines error handling as following. > The SQL operators JSON_VALUE, JSON_QUERY, JSON_TABLE, and JSON_EXISTS provide > the following mechanisms to handle these errors: > 1) The SQL/JSON path language traps any errors that occur during the > evaluation > of a . Depending on the precise > contained in the , the result may be Unknown, True, or > False, depending on the outcome of non-error tests evaluated in the predicate>. > 2) The SQL/JSON path language has two modes, strict and lax, which govern > structural errors, as follows: > a) In lax mode: > i) If an operation requires an SQL/JSON array but the operand is not an > SQL > JSON array, then the operand is first “wrapped” in an SQL/JSON array prior > to performing the operation. > ii) If an operation requires something other than an SQL/JSON array, but > the operand is an SQL/JSON array, then the operand is “unwrapped” by > converting its elements into an SQL/JSON sequence prior to performing the > operation. > iii) After applying the preceding resolutions to structural errors, if > there is still a structural error, the result is an empty SQL/JSON > sequence. > b) In strict mode, if the structural error occurs within aexpression>, then the error handling of applies >Otherwise, a structural error is an unhandled error. > 3) Non-structural errors outside of a are always > unhandled errors, resulting in an exception condition returned from the path > engine to the SQL/JSON query operator. > 4) The SQL/JSON query operators provide an ON ERROR clause to specify the > behavior in case of an input conversion error, an unhandled structural error, > an unhandled non-structural error, or an output conversion error. So, basically standard requires us to suppress any error happening in filter expression. But as I wrote before suppression of errors in datetime comparison may lead to surprising results. That happens in rare corner cases, but still. This makes uneasy choice between consistent behavior and standard behavior. However, Nikita Glukhov gave to good idea about that. Instead on thinking about whether we should suppress or not cast errors in datetime comparison, we may just eliminate those error. So, if we know that casting date to timestamp overflows upper bound of finite timestamp, then we also know that this date is greater than any finite timestamp. So, we still able to do correct comparison. I'm going to implement this and post a patch. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company On Mon, Sep 30, 2019 at 10:56 PM Robert Haas wrote: > > On Sun, Sep 29, 2019 at 10:30 AM Alexander Korotkov > wrote: > > So, jsonpath behaves like 100 is not greater than 2020. This > > looks like plain false. And user can't expect that unless she is > > famili
Re: pgsql: Implement jsonpath .datetime() method
On Fri, Sep 27, 2019 at 6:58 PM Nikita Glukhov wrote:
> On Thu, Sep 26, 2019 at 2:57 AM Tom Lane
> wrote:
> > * More generally, it's completely unclear why some error conditions
> > are thrown as errors and others just result in returning *have_error.
> > In particular, it seems weird that some unsupported datatype combinations
> > cause hard errors while others do not. Maybe that's fine, but if so,
> > the function header comment is falling down on the job by not explaining
> > the reasoning.
>
> All cast errors are caught by jsonpath predicate. Comparison of the
> uncomparable datetime types (time[tz] to dated types) also returns Unknown.
> And only if datatype conversion requires current timezone, which is not
> available in immutable family of jsonb_xxx() functions, hard error is thrown.
> This behavior is specific only for our jsonpath implementation. But I'm
> really not sure if we should throw an error or return Unknown in this case.
I'd like to share my further thoughts about errors. I think we should
suppress errors defined by standard and which user can expect. So,
user can expect that wrong date format causes an error, division by
zero causes an error and so on. And those errors are defined by
standard.
However, we error is caused by limitation of our implementation, then
suppression doesn't look right to me.
For instance.
# select jsonb_path_query('"100-01-01"', '$.datetime() >
"2020-01-01 12:00:00".datetime()'::jsonpath);
jsonb_path_query
--
null
(1 row)
# select '100-01-01'::date > '2020-01-01 12:00:00'::timestamp;
ERROR: date out of range for timestamp
So, jsonpath behaves like 100 is not greater than 2020. This
looks like plain false. And user can't expect that unless she is
familiar with our particular issues. Now I got opinion that such
errors shouldn't be suppressed. We can't suppress *every* error. If
trying to do this, we can come to an idea to suppress OOM error and
return garbage then, which is obviously ridiculous. Opinions?
--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Re: pgsql: Implement jsonpath .datetime() method
On Thu, Sep 26, 2019 at 2:57 AM Tom Lane wrote: > Alexander Korotkov writes: > > On Thu, Sep 26, 2019 at 2:12 AM Tom Lane wrote: > >> The proximate problem seems to be that compareItems() is insufficiently > >> careful to ensure that both values are non-null before passing them > >> off to datatype-specific code. The code accidentally fails to crash > >> on 64-bit machines, but it's still giving garbage answers, I think. > > > I've found compareItems() code to not apply appropriate cast to/from > > Datum. Fixed in 7881bb14f4. This makes test pass on my local 32-bit > > machine. I'll keep look on buildfarm. > > Hm. dromedary seems not to crash either with that fix, but I'm not > sure why not, because when I was running the previous tree by hand, > the stack trace showed pretty clearly that we were getting to > timestamp_cmp with one null and one non-null argument. So I don't > believe your argument that that's impossible, and even if it is, > I do not think it's sane for compareItems to depend on that --- > especially when one of its code paths *does* check for nulls. > > I do not have a very good opinion about the quality of this code > upon my first glance at it. Just looking at compareDatetime: > > * The code is schizophrenic about whether it's allowed to pass a > null have_error pointer or not. It is not very sensible to have > some code doing > if (have_error && *have_error) > return 0; > when other code branches will dump core for null have_error. > Given that if this test actually was doing anything, what it > would be doing is failing to detect error conditions, I think > the only sensible design is to insist that have_error mustn't be > null, in which case these checks for null pointer should be removed, > because (a) they waste code and cycles and (b) they mislead the > reader as to what the API of compareDatetime actually is. > > * At least some of the code paths will malfunction if the caller > didn't initialize *have_error to false. If that is an intended API > requirement, it is not acceptable for the function header comment > not to say so. (For bonus points, it'd be nice if the header > comment agreed with the code as to the name of the variable.) > If this isn't an intended requirement, you need to fix the code, > probably by initializing "*have_error = false;" up at the top. > > * It's a bit schizophrenic also that some of the switches > lack default:s (and rely on the if (!cmpfunc) below), while > the outer switch does have its own, completely redundant > default:. I'd get rid of that default: and instead add > a comment explaining that the !cmpfunc test substitutes for > default branches. > > * This is silly: > > if (*have_error) > return 0; > > *have_error = false; > > * Also, given that you have that "if (*have_error)" where you do, > the have_error tests inside the switch are useless redundancy. > You might as well just remove them completely and let the final > test handle falling out if a conversion failed. Alternatively > you could drop the final test, because as the code stands right > now, it's visibly impossible to get there with *have_error true. > > * More generally, it's completely unclear why some error conditions > are thrown as errors and others just result in returning *have_error. > In particular, it seems weird that some unsupported datatype combinations > cause hard errors while others do not. Maybe that's fine, but if so, > the function header comment is falling down on the job by not explaining > the reasoning. > > * OIDs are unsigned, so if you must print them, use %u not %d. > > * The errhints don't follow project message style. > > * The blank lines before "break"s aren't really per project > style either, IMO. They certainly aren't doing anything to > improve readability, and they help limit how much code you > can see at once. Thank you for feedback. Nikita and me will provide patches to fix pointed issues during next couple of days. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: pgsql: Implement jsonpath .datetime() method
On Thu, Sep 26, 2019 at 2:12 AM Tom Lane wrote: > Alexander Korotkov writes: > > I've noticed buildfarm is unhappy with my commits. > > The proximate problem seems to be that compareItems() is insufficiently > careful to ensure that both values are non-null before passing them > off to datatype-specific code. The code accidentally fails to crash > on 64-bit machines, but it's still giving garbage answers, I think. I've found compareItems() code to not apply appropriate cast to/from Datum. Fixed in 7881bb14f4. This makes test pass on my local 32-bit machine. I'll keep look on buildfarm. It seems that caller (executePredicate()) don't NULL values to compareItems(). It does do only for predicates, which doesn't have right argument. But binary predicate with lacking argument should be detected during syntactic analysis. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
pgsql: Correctly cast types to Datum and back in compareDatetime()
Correctly cast types to Datum and back in compareDatetime() Discussion: https://postgr.es/m/CAPpHfdteFKW6MLpXM4md99m55YAuXs0n9_P2wiTq_EmG09doUA%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/7881bb14f4b23e8dc8671938cfb3f34117c12d8b Modified Files -- src/backend/utils/adt/jsonpath_exec.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-)
Re: pgsql: Implement jsonpath .datetime() method
On Thu, Sep 26, 2019 at 12:05 AM Alexander Korotkov < a.korot...@postgrespro.ru> wrote: > > On Wed, Sep 25, 2019 at 10:52 PM Alexander Korotkov > wrote: > > Implement jsonpath .datetime() method > > I've noticed buildfarm is unhappy with my commits. > > 1. https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing=2019-09-25%2020%3A40%3A11 > 2. https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dromedary=2019-09-25%2020%3A38%3A21 Appears to be another issue happening when USE_FLOAT8_BYVAL is undefined. Managed to reproduce it locally. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: pgsql: Implement jsonpath .datetime() method
On Wed, Sep 25, 2019 at 10:52 PM Alexander Korotkov wrote: > Implement jsonpath .datetime() method I've noticed buildfarm is unhappy with my commits. 1. https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing=2019-09-25%2020%3A40%3A11 2. https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dromedary=2019-09-25%2020%3A38%3A21 I'm investigating the issue. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
pgsql: Error suppression support for upcoming jsonpath .datetime() meth
Error suppression support for upcoming jsonpath .datetime() method Add support of error suppression in some date and time manipulation functions as it's required for jsonpath .datetime() method support. This commit doesn't use PG_TRY()/PG_CATCH() in order to implement that. Instead, it provides internal versions of date and time functions used, which support error suppression. Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com Author: Alexander Korotkov, Nikita Glukhov Reviewed-by: Anastasia Lubennikova, Peter Eisentraut Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/5bc450629b31a0b6986e668056d5bd36792412d2 Modified Files -- src/backend/utils/adt/date.c | 86 -- src/backend/utils/adt/formatting.c | 537 - src/backend/utils/adt/timestamp.c | 66 +++-- src/include/utils/date.h | 2 + src/include/utils/datetime.h | 2 + src/include/utils/formatting.h | 3 +- src/include/utils/timestamp.h | 3 + 7 files changed, 479 insertions(+), 220 deletions(-)
pgsql: Implement standard datetime parsing mode
Implement standard datetime parsing mode SQL Standard 2016 defines rules for handling separators in datetime template strings, which are different to to_date()/to_timestamp() rules. Standard allows only small set of separators and requires strict matching for them. Standard applies to jsonpath .datetime() method and CAST (... FORMAT ...) SQL clause. We're not going to change handling of separators in existing to_date()/to_timestamp() functions, because their current behavior is familiar for users. Standard behavior now available by special flag, which will be used in upcoming .datetime() jsonpath method. Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com Author: Alexander Korotkov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/1a950f37d0a283f2a76bec63c05530ed6eb16de1 Modified Files -- src/backend/utils/adt/formatting.c | 144 ++--- 1 file changed, 104 insertions(+), 40 deletions(-)
pgsql: Implement jsonpath .datetime() method
Implement jsonpath .datetime() method This commit implements jsonpath .datetime() method as it's specified in SQL/JSON standard. There are no-argument and single-argument versions of this method. No-argument version selects first of ISO datetime formats matching input string. Single-argument version accepts template string as its argument. Additionally to .datetime() method itself this commit also implements comparison ability of resulting date and time values. There is some difficulty because exising jsonb_path_*() functions are immutable, while comparison of timezoned and non-timezoned types involves current timezone. At first, current timezone could be changes in session. Moreover, timezones themselves are not immutable and could be updated. This is why we let existing immutable functions throw errors on such non-immutable comparison. In the same time this commit provides jsonb_path_*_tz() functions which are stable and support operations involving timezones. As new functions are added to the system catalog, catversion is bumped. Support of .datetime() method was the only blocker prevents T832 from being marked as supported. sql_features.txt is updated correspondingly. Extracted from original patch by Nikita Glukhov, Teodor Sigaev, Oleg Bartunov. Heavily revised by me. Comments were adjusted by Liudmila Mantrova. Discussion: https://postgr.es/m/fcc6fc6a-b497-f39a-923d-aa34d0c588e8%402ndQuadrant.com Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com Author: Alexander Korotkov, Nikita Glukhov, Teodor Sigaev, Oleg Bartunov, Liudmila Mantrova Reviewed-by: Anastasia Lubennikova, Peter Eisentraut Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/bffe1bd68457e43925c362d8728ce3b25bdf1c94 Modified Files -- doc/src/sgml/func.sgml | 117 +- src/backend/catalog/sql_features.txt | 2 +- src/backend/catalog/system_views.sql | 40 +++ src/backend/utils/adt/jsonpath.c | 24 +- src/backend/utils/adt/jsonpath_exec.c| 470 ++-- src/backend/utils/adt/jsonpath_gram.y| 14 + src/backend/utils/adt/jsonpath_scan.l| 1 + src/backend/utils/errcodes.txt | 1 + src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 22 ++ src/include/utils/jsonpath.h | 1 + src/test/regress/expected/jsonb_jsonpath.out | 520 +++ src/test/regress/expected/jsonpath.out | 12 + src/test/regress/sql/jsonb_jsonpath.sql | 172 + src/test/regress/sql/jsonpath.sql| 2 + 15 files changed, 1355 insertions(+), 45 deletions(-)
pgsql: Allow datetime values in JsonbValue
Allow datetime values in JsonbValue SQL/JSON standard allows manipulation with datetime values. So, it appears to be convinient to allow datetime values to be represented in JsonbValue struct. These datetime values are allowed for temporary representation only. During serialization datetime values are converted into strings. SQL/JSON requires writing timestamps with timezone in the same timezone offset as they were parsed. This is why we allow storage of timezone offset in JsonbValue struct. For the same reason timezone offset argument is added to JsonEncodeDateTime() function. Extracted from original patch by Nikita Glukhov, Teodor Sigaev, Oleg Bartunov. Revised by me. Comments were adjusted by Liudmila Mantrova. Discussion: https://postgr.es/m/fcc6fc6a-b497-f39a-923d-aa34d0c588e8%402ndQuadrant.com Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com Author: Nikita Glukhov, Teodor Sigaev, Oleg Bartunov, Alexander Korotkov, Liudmila Mantrova Reviewed-by: Anastasia Lubennikova, Peter Eisentraut Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/6dda292d4df82a9158d1acc93feecf3b84637b59 Modified Files -- src/backend/utils/adt/json.c | 32 ++-- src/backend/utils/adt/jsonb.c | 27 --- src/backend/utils/adt/jsonb_util.c | 21 + src/include/utils/jsonapi.h| 3 ++- src/include/utils/jsonb.h | 24 +--- 5 files changed, 94 insertions(+), 13 deletions(-)
pgsql: Implement parse_datetime() function
Implement parse_datetime() function This commit adds parse_datetime() function, which implements datetime parsing with extended features demanded by upcoming jsonpath .datetime() method: * Dynamic type identification based on template string, * Support for standard-conforming 'strict' mode, * Timezone offset is returned as separate value. Extracted from original patch by Nikita Glukhov, Teodor Sigaev, Oleg Bartunov. Revised by me. Discussion: https://postgr.es/m/fcc6fc6a-b497-f39a-923d-aa34d0c588e8%402ndQuadrant.com Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com Author: Nikita Glukhov, Teodor Sigaev, Oleg Bartunov, Alexander Korotkov Reviewed-by: Anastasia Lubennikova, Peter Eisentraut Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/66c74f8b6e347ba5830bf06468bef8081601c187 Modified Files -- src/backend/utils/adt/date.c | 11 +- src/backend/utils/adt/formatting.c | 291 - src/include/utils/date.h | 3 + src/include/utils/formatting.h | 3 + 4 files changed, 296 insertions(+), 12 deletions(-)
pgsql: Fix bug in pairingheap_SpGistSearchItem_cmp()
Fix bug in pairingheap_SpGistSearchItem_cmp() Our item contains only so->numberOfNonNullOrderBys of distances. Reflect that in the loop upper bound. Discussion: https://postgr.es/m/53536807-784c-e029-6e92-6da802ab8d60%40postgrespro.ru Author: Nikita Glukhov Backpatch-through: 12 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/90c0987258264de07780f0329db2fce83098fba8 Modified Files -- src/backend/access/spgist/spgscan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Fix bug in pairingheap_SpGistSearchItem_cmp()
Fix bug in pairingheap_SpGistSearchItem_cmp() Our item contains only so->numberOfNonNullOrderBys of distances. Reflect that in the loop upper bound. Discussion: https://postgr.es/m/53536807-784c-e029-6e92-6da802ab8d60%40postgrespro.ru Author: Nikita Glukhov Backpatch-through: 12 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/8728552b291b8fe3778346fb4d4b7d1c8743f708 Modified Files -- src/backend/access/spgist/spgscan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Provide stable test for NULL-values in KNN SP-GiST
Provide stable test for NULL-values in KNN SP-GiST f5f084fc3e has removed test because of its instability. This commit provides alternative test with determined ordering using extra ORDER BY expression. Backpatch-through: 12 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/d75386a3fd34d8902667474318c2ebe40978de32 Modified Files -- src/test/regress/expected/create_index_spgist.out | 10 ++ src/test/regress/sql/create_index_spgist.sql | 4 2 files changed, 14 insertions(+)
pgsql: Provide stable test for NULL-values in KNN SP-GiST
Provide stable test for NULL-values in KNN SP-GiST f5f084fc3e has removed test because of its instability. This commit provides alternative test with determined ordering using extra ORDER BY expression. Backpatch-through: 12 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/5033e9580869fec514d787dc9d3b0b63cce0bcfb Modified Files -- src/test/regress/expected/create_index_spgist.out | 10 ++ src/test/regress/sql/create_index_spgist.sql | 4 2 files changed, 14 insertions(+)
pgsql: Remove unstable KNN SP-GiST test
Remove unstable KNN SP-GiST test 6cae9d2c10 introduced test for NULL values in KNN SP-GiST. This test relies on undetermined ordering showing different results on various platforms. This commit removes that test. Will be replaced with better test later. Discussion: https://postgr.es/m/6d51305e1159241cabee132f7efc7eff%40xs4all.nl Backpatch-through: 12 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/340536dd489286d0f98878843390d92ddc593be0 Modified Files -- src/test/regress/expected/create_index_spgist.out | 10 -- src/test/regress/sql/create_index_spgist.sql | 4 2 files changed, 14 deletions(-)
pgsql: Remove unstable KNN SP-GiST test
Remove unstable KNN SP-GiST test 6cae9d2c10 introduced test for NULL values in KNN SP-GiST. This test relies on undetermined ordering showing different results on various platforms. This commit removes that test. Will be replaced with better test later. Discussion: https://postgr.es/m/6d51305e1159241cabee132f7efc7eff%40xs4all.nl Backpatch-through: 12 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/f5f084fc3ec516545d826e1e9b7ab4aabf612698 Modified Files -- src/test/regress/expected/create_index_spgist.out | 10 -- src/test/regress/sql/create_index_spgist.sql | 4 2 files changed, 14 deletions(-)
pgsql: Fix freeing old values in index_store_float8_orderby_distances()
Fix freeing old values in index_store_float8_orderby_distances() 6cae9d2c10 has added an error in freeing old values in index_store_float8_orderby_distances() function. It looks for old value in scan->xs_orderbynulls[i] after setting a new value there. This commit fixes that. Also it removes short-circuit in handling distances == NULL situation. Now distances == NULL will be treated the same way as array with all null distances. That is, previous values will be freed if any. Reported-by: Tom Lane, Nikita Glukhov Discussion: https://postgr.es/m/CAPpHfdu2wcoAVAm3Ek66rP%3Duo_C-D84%2B%2Buf1VEcbyi_caBXWCA%40mail.gmail.com Discussion: https://postgr.es/m/426580d3-a668-b9d1-7b8e-f74d1a6524e0%40postgrespro.ru Backpatch-through: 12 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/8c58e480d9ddecbe94badc737d60c866974144b5 Modified Files -- src/backend/access/index/indexam.c | 40 +++--- 1 file changed, 20 insertions(+), 20 deletions(-)
pgsql: Fix freeing old values in index_store_float8_orderby_distances()
Fix freeing old values in index_store_float8_orderby_distances() 6cae9d2c10 has added an error in freeing old values in index_store_float8_orderby_distances() function. It looks for old value in scan->xs_orderbynulls[i] after setting a new value there. This commit fixes that. Also it removes short-circuit in handling distances == NULL situation. Now distances == NULL will be treated the same way as array with all null distances. That is, previous values will be freed if any. Reported-by: Tom Lane, Nikita Glukhov Discussion: https://postgr.es/m/CAPpHfdu2wcoAVAm3Ek66rP%3Duo_C-D84%2B%2Buf1VEcbyi_caBXWCA%40mail.gmail.com Discussion: https://postgr.es/m/426580d3-a668-b9d1-7b8e-f74d1a6524e0%40postgrespro.ru Backpatch-through: 12 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/8c8a267201bebb0edeaab2a76b7d3bcc9739924f Modified Files -- src/backend/access/index/indexam.c | 40 +++--- 1 file changed, 20 insertions(+), 20 deletions(-)
Re: pgsql: Improve handling of NULLs in KNN-GiST and KNN-SP-GiST
On Thu, Sep 19, 2019 at 11:43 PM Alexander Korotkov wrote: > On Thu, Sep 19, 2019 at 11:31 PM Alexander Korotkov > wrote: > > > > On Thu, Sep 19, 2019 at 11:13 PM Tom Lane wrote: > >> > >> Erik Rijkers writes: > >> > Oops: > >> > create_index ... ok 634 ms > >> > create_index_spgist ... FAILED 438 ms > >> > create_view ... ok 329 ms > >> > >> I'm betting the issue is breaking the Datum abstraction here: > >> > >> - scan->xs_orderbyvals[i] = > >> Float8GetDatum(distanceValues[i]); > >> + scan->xs_orderbyvals[i] = item->distances[i].value; > > > > > > Overseen by me. Will fix immediately. > > > Fix pushed from 11 to 9.5, where I made this error during backpatching. > > However, I also see set of failures in master, which seems related to > this patch: > * > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dromedary=2019-09-19%2020%3A09%3A42 > * > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake=2019-09-19%2020%3A04%3A22 > * > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=tern=2019-09-19%2019%3A22%3A01 > > Will investigate them. Both dromedary and tern, where segfault happened, are 32-bit. Bug seems related to USE_FLOAT8_BYVAL or something. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: pgsql: Improve handling of NULLs in KNN-GiST and KNN-SP-GiST
On Thu, Sep 19, 2019 at 11:31 PM Alexander Korotkov wrote: > > On Thu, Sep 19, 2019 at 11:13 PM Tom Lane wrote: >> >> Erik Rijkers writes: >> > Oops: >> > create_index ... ok 634 ms >> > create_index_spgist ... FAILED 438 ms >> > create_view ... ok 329 ms >> >> I'm betting the issue is breaking the Datum abstraction here: >> >> - scan->xs_orderbyvals[i] = >> Float8GetDatum(distanceValues[i]); >> + scan->xs_orderbyvals[i] = item->distances[i].value; > > > Overseen by me. Will fix immediately. Fix pushed from 11 to 9.5, where I made this error during backpatching. However, I also see set of failures in master, which seems related to this patch: * https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dromedary=2019-09-19%2020%3A09%3A42 * https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake=2019-09-19%2020%3A04%3A22 * https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=tern=2019-09-19%2019%3A22%3A01 Will investigate them. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
pgsql: Fix oversight in backpatch of 6cae9d2c10
Fix oversight in backpatch of 6cae9d2c10 During backpatch of 6cae9d2c10 Float8GetDatum() was accidentally removed. This commit turns it back. Reported-by: Erik Rijkers Discussion: https://postgr.es/m/6d51305e1159241cabee132f7efc7eff%40xs4all.nl Author: Tom Lane Backpatch-through: from 11 to 9.5 Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/388939748a7683b1e703de8d11ead60b0be3edc7 Modified Files -- src/backend/access/gist/gistget.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Fix oversight in backpatch of 6cae9d2c10
Fix oversight in backpatch of 6cae9d2c10 During backpatch of 6cae9d2c10 Float8GetDatum() was accidentally removed. This commit turns it back. Reported-by: Erik Rijkers Discussion: https://postgr.es/m/6d51305e1159241cabee132f7efc7eff%40xs4all.nl Author: Tom Lane Backpatch-through: from 11 to 9.5 Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/984b9ba1d119ee59babea515b2ecab70b7b11910 Modified Files -- src/backend/access/gist/gistget.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Fix oversight in backpatch of 6cae9d2c10
Fix oversight in backpatch of 6cae9d2c10 During backpatch of 6cae9d2c10 Float8GetDatum() was accidentally removed. This commit turns it back. Reported-by: Erik Rijkers Discussion: https://postgr.es/m/6d51305e1159241cabee132f7efc7eff%40xs4all.nl Author: Tom Lane Backpatch-through: from 11 to 9.5 Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/2f0434e8e96f8850705fbd46a12fb5286cf8922a Modified Files -- src/backend/access/gist/gistget.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Fix oversight in backpatch of 6cae9d2c10
Fix oversight in backpatch of 6cae9d2c10 During backpatch of 6cae9d2c10 Float8GetDatum() was accidentally removed. This commit turns it back. Reported-by: Erik Rijkers Discussion: https://postgr.es/m/6d51305e1159241cabee132f7efc7eff%40xs4all.nl Author: Tom Lane Backpatch-through: from 11 to 9.5 Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/140b7b1f93dcd681bf95fef4e3d9b71289c9af87 Modified Files -- src/backend/access/gist/gistget.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Re: pgsql: Improve handling of NULLs in KNN-GiST and KNN-SP-GiST
On Thu, Sep 19, 2019 at 11:13 PM Tom Lane wrote: > Erik Rijkers writes: > > Oops: > > create_index ... ok 634 ms > > create_index_spgist ... FAILED 438 ms > > create_view ... ok 329 ms > > I'm betting the issue is breaking the Datum abstraction here: > > - scan->xs_orderbyvals[i] = > Float8GetDatum(distanceValues[i]); > + scan->xs_orderbyvals[i] = item->distances[i].value; > Overseen by me. Will fix immediately. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
pgsql: Improve handling of NULLs in KNN-GiST and KNN-SP-GiST
Improve handling of NULLs in KNN-GiST and KNN-SP-GiST This commit improves subject in two ways: * It removes ugliness of 02f90879e7, which stores distance values and null flags in two separate arrays after GISTSearchItem struct. Instead we pack both distance value and null flag in IndexOrderByDistance struct. Alignment overhead should be negligible, because we typically deal with at most few "col op const" expressions in ORDER BY clause. * It fixes handling of "col op NULL" expression in KNN-SP-GiST. Now, these expression are not passed to support functions, which can't deal with them. Instead, NULL result is implicitly assumed. It future we may decide to teach support functions to deal with NULL arguments, but current solution is bugfix suitable for backpatch. Reported-by: Nikita Glukhov Discussion: https://postgr.es/m/826f57ee-afc7-8977-c44c-6111d18b02ec%40postgrespro.ru Author: Nikita Glukhov Reviewed-by: Alexander Korotkov Backpatch-through: 9.4 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/6cae9d2c10e151f741e7bc64a8b70bb2615c367c Modified Files -- src/backend/access/gist/gistget.c | 68 - src/backend/access/gist/gistscan.c| 16 ++--- src/backend/access/index/indexam.c| 22 +++ src/backend/access/spgist/spgscan.c | 74 +++ src/include/access/genam.h| 10 ++- src/include/access/gist_private.h | 27 ++--- src/include/access/spgist_private.h | 8 ++- src/test/regress/expected/create_index_spgist.out | 10 +++ src/test/regress/sql/create_index_spgist.sql | 5 ++ src/tools/pgindent/typedefs.list | 1 + 10 files changed, 140 insertions(+), 101 deletions(-)
pgsql: Improve handling of NULLs in KNN-GiST and KNN-SP-GiST
Improve handling of NULLs in KNN-GiST and KNN-SP-GiST This commit improves subject in two ways: * It removes ugliness of 02f90879e7, which stores distance values and null flags in two separate arrays after GISTSearchItem struct. Instead we pack both distance value and null flag in IndexOrderByDistance struct. Alignment overhead should be negligible, because we typically deal with at most few "col op const" expressions in ORDER BY clause. * It fixes handling of "col op NULL" expression in KNN-SP-GiST. Now, these expression are not passed to support functions, which can't deal with them. Instead, NULL result is implicitly assumed. It future we may decide to teach support functions to deal with NULL arguments, but current solution is bugfix suitable for backpatch. Reported-by: Nikita Glukhov Discussion: https://postgr.es/m/826f57ee-afc7-8977-c44c-6111d18b02ec%40postgrespro.ru Author: Nikita Glukhov Reviewed-by: Alexander Korotkov Backpatch-through: 9.4 Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/53d9cf2db5ded873fbcc031acb584a2c05e4983b Modified Files -- src/backend/access/gist/gistget.c | 75 ++ src/backend/access/gist/gistscan.c | 16 +++- src/include/access/genam.h | 7 src/include/access/gist_private.h | 27 +++--- src/tools/pgindent/typedefs.list | 1 + 5 files changed, 47 insertions(+), 79 deletions(-)
pgsql: Improve handling of NULLs in KNN-GiST and KNN-SP-GiST
Improve handling of NULLs in KNN-GiST and KNN-SP-GiST This commit improves subject in two ways: * It removes ugliness of 02f90879e7, which stores distance values and null flags in two separate arrays after GISTSearchItem struct. Instead we pack both distance value and null flag in IndexOrderByDistance struct. Alignment overhead should be negligible, because we typically deal with at most few "col op const" expressions in ORDER BY clause. * It fixes handling of "col op NULL" expression in KNN-SP-GiST. Now, these expression are not passed to support functions, which can't deal with them. Instead, NULL result is implicitly assumed. It future we may decide to teach support functions to deal with NULL arguments, but current solution is bugfix suitable for backpatch. Reported-by: Nikita Glukhov Discussion: https://postgr.es/m/826f57ee-afc7-8977-c44c-6111d18b02ec%40postgrespro.ru Author: Nikita Glukhov Reviewed-by: Alexander Korotkov Backpatch-through: 9.4 Branch -- REL9_4_STABLE Details --- https://git.postgresql.org/pg/commitdiff/332eda5bd3d275fc14b721905b93c2b983a000d3 Modified Files -- src/backend/access/gist/gistget.c | 65 ++ src/backend/access/gist/gistscan.c | 16 -- src/include/access/genam.h | 7 src/include/access/gist_private.h | 28 src/tools/pgindent/typedefs.list | 1 + 5 files changed, 44 insertions(+), 73 deletions(-)
pgsql: Improve handling of NULLs in KNN-GiST and KNN-SP-GiST
Improve handling of NULLs in KNN-GiST and KNN-SP-GiST This commit improves subject in two ways: * It removes ugliness of 02f90879e7, which stores distance values and null flags in two separate arrays after GISTSearchItem struct. Instead we pack both distance value and null flag in IndexOrderByDistance struct. Alignment overhead should be negligible, because we typically deal with at most few "col op const" expressions in ORDER BY clause. * It fixes handling of "col op NULL" expression in KNN-SP-GiST. Now, these expression are not passed to support functions, which can't deal with them. Instead, NULL result is implicitly assumed. It future we may decide to teach support functions to deal with NULL arguments, but current solution is bugfix suitable for backpatch. Reported-by: Nikita Glukhov Discussion: https://postgr.es/m/826f57ee-afc7-8977-c44c-6111d18b02ec%40postgrespro.ru Author: Nikita Glukhov Reviewed-by: Alexander Korotkov Backpatch-through: 9.4 Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/2da8e56db35cf3759c22b97bcb22216837dec3e1 Modified Files -- src/backend/access/gist/gistget.c | 75 ++ src/backend/access/gist/gistscan.c | 16 +++- src/include/access/genam.h | 7 src/include/access/gist_private.h | 27 +++--- src/tools/pgindent/typedefs.list | 1 + 5 files changed, 47 insertions(+), 79 deletions(-)
pgsql: Improve handling of NULLs in KNN-GiST and KNN-SP-GiST
Improve handling of NULLs in KNN-GiST and KNN-SP-GiST This commit improves subject in two ways: * It removes ugliness of 02f90879e7, which stores distance values and null flags in two separate arrays after GISTSearchItem struct. Instead we pack both distance value and null flag in IndexOrderByDistance struct. Alignment overhead should be negligible, because we typically deal with at most few "col op const" expressions in ORDER BY clause. * It fixes handling of "col op NULL" expression in KNN-SP-GiST. Now, these expression are not passed to support functions, which can't deal with them. Instead, NULL result is implicitly assumed. It future we may decide to teach support functions to deal with NULL arguments, but current solution is bugfix suitable for backpatch. Reported-by: Nikita Glukhov Discussion: https://postgr.es/m/826f57ee-afc7-8977-c44c-6111d18b02ec%40postgrespro.ru Author: Nikita Glukhov Reviewed-by: Alexander Korotkov Backpatch-through: 9.4 Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/ad458d0cd9d90df4747b48ec60cfde455e878811 Modified Files -- src/backend/access/gist/gistget.c | 75 ++ src/backend/access/gist/gistscan.c | 16 +++- src/include/access/genam.h | 7 src/include/access/gist_private.h | 28 -- src/tools/pgindent/typedefs.list | 1 + 5 files changed, 48 insertions(+), 79 deletions(-)
pgsql: Improve handling of NULLs in KNN-GiST and KNN-SP-GiST
Improve handling of NULLs in KNN-GiST and KNN-SP-GiST This commit improves subject in two ways: * It removes ugliness of 02f90879e7, which stores distance values and null flags in two separate arrays after GISTSearchItem struct. Instead we pack both distance value and null flag in IndexOrderByDistance struct. Alignment overhead should be negligible, because we typically deal with at most few "col op const" expressions in ORDER BY clause. * It fixes handling of "col op NULL" expression in KNN-SP-GiST. Now, these expression are not passed to support functions, which can't deal with them. Instead, NULL result is implicitly assumed. It future we may decide to teach support functions to deal with NULL arguments, but current solution is bugfix suitable for backpatch. Reported-by: Nikita Glukhov Discussion: https://postgr.es/m/826f57ee-afc7-8977-c44c-6111d18b02ec%40postgrespro.ru Author: Nikita Glukhov Reviewed-by: Alexander Korotkov Backpatch-through: 9.4 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/31cbd760578fc0aeb87db11422c32afaf5de129b Modified Files -- src/backend/access/gist/gistget.c | 68 - src/backend/access/gist/gistscan.c| 16 ++--- src/backend/access/index/indexam.c| 22 +++ src/backend/access/spgist/spgscan.c | 74 +++ src/include/access/genam.h| 10 ++- src/include/access/gist_private.h | 27 ++--- src/include/access/spgist_private.h | 8 ++- src/test/regress/expected/create_index_spgist.out | 10 +++ src/test/regress/sql/create_index_spgist.sql | 5 ++ src/tools/pgindent/typedefs.list | 1 + 10 files changed, 140 insertions(+), 101 deletions(-)
pgsql: Improve handling of NULLs in KNN-GiST and KNN-SP-GiST
Improve handling of NULLs in KNN-GiST and KNN-SP-GiST This commit improves subject in two ways: * It removes ugliness of 02f90879e7, which stores distance values and null flags in two separate arrays after GISTSearchItem struct. Instead we pack both distance value and null flag in IndexOrderByDistance struct. Alignment overhead should be negligible, because we typically deal with at most few "col op const" expressions in ORDER BY clause. * It fixes handling of "col op NULL" expression in KNN-SP-GiST. Now, these expression are not passed to support functions, which can't deal with them. Instead, NULL result is implicitly assumed. It future we may decide to teach support functions to deal with NULL arguments, but current solution is bugfix suitable for backpatch. Reported-by: Nikita Glukhov Discussion: https://postgr.es/m/826f57ee-afc7-8977-c44c-6111d18b02ec%40postgrespro.ru Author: Nikita Glukhov Reviewed-by: Alexander Korotkov Backpatch-through: 9.4 Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/d6a90aac563ecb5b32e2a1d902c9a18d8c21671f Modified Files -- src/backend/access/gist/gistget.c | 75 ++ src/backend/access/gist/gistscan.c | 16 +++- src/include/access/genam.h | 7 src/include/access/gist_private.h | 27 +++--- src/tools/pgindent/typedefs.list | 1 + 5 files changed, 47 insertions(+), 79 deletions(-)
pgsql: Support for SSSSS datetime format pattern
Support for S datetime format pattern SQL Standard 2016 defines S format pattern for seconds past midnight in jsonpath .datetime() method and CAST (... FORMAT ...) SQL clause. In our datetime parsing engine we currently support it with name. This commit adds S as an alias for . Alias is added in favor of upcoming jsonpath .datetime() method. But it's also supported in to_date()/ to_timestamp() as positive side effect. Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com Author: Nikita Glukhov, Alexander Korotkov Reviewed-by: Anastasia Lubennikova, Peter Eisentraut Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/b64b857f50fb51da1588c54a56f8fc1c0d491058 Modified Files -- doc/src/sgml/func.sgml | 2 +- src/backend/utils/adt/formatting.c | 12 src/test/regress/expected/horology.out | 20 src/test/regress/sql/horology.sql | 4 4 files changed, 33 insertions(+), 5 deletions(-)
pgsql: Support for FF1-FF6 datetime format patterns
Support for FF1-FF6 datetime format patterns SQL Standard 2016 defines FF1-FF9 format patters for fractions of seconds in jsonpath .datetime() method and CAST (... FORMAT ...) SQL clause. Parsing engine of upcoming .datetime() method will be shared with to_date()/ to_timestamp(). This patch implements FF1-FF6 format patterns for upcoming jsonpath .datetime() method. to_date()/to_timestamp() functions will also get support of this format patterns as positive side effect. FF7-FF9 are not supported due to lack of precision in our internal timestamp representation. Extracted from original patch by Nikita Glukhov, Teodor Sigaev, Oleg Bartunov. Heavily revised by me. Discussion: https://postgr.es/m/fcc6fc6a-b497-f39a-923d-aa34d0c588e8%402ndQuadrant.com Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com Author: Nikita Glukhov, Teodor Sigaev, Oleg Bartunov, Alexander Korotkov Reviewed-by: Anastasia Lubennikova, Peter Eisentraut Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/d589f94460c24d9b7ac21887d031818d6e3f354d Modified Files -- doc/src/sgml/func.sgml| 24 src/backend/utils/adt/formatting.c| 99 --- src/backend/utils/adt/timestamp.c | 3 +- src/include/utils/datetime.h | 2 + src/test/regress/expected/horology.out| 79 src/test/regress/expected/timestamp.out | 15 + src/test/regress/expected/timestamptz.out | 15 + src/test/regress/sql/horology.sql | 9 +++ src/test/regress/sql/timestamp.sql| 8 +++ src/test/regress/sql/timestamptz.sql | 8 +++ 10 files changed, 239 insertions(+), 23 deletions(-)
pgsql: Typo fixes for documentation
Typo fixes for documentation Discussion: https://postgr.es/m/CAEkD-mDUZrRE%3Dk-FznEg4Ed2VdjpZCyHoyo%2Bp0%2B8KvHqR%3DpNVQ%40mail.gmail.com Author: Liudmila Mantrova, Alexander Lakhin Reviewed-by: Alexander Korotkov, Alvaro Herrera Backpatch-through: 12 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/6eb9b20bb7dd770c78bf43b205b98e15972e2341 Modified Files -- doc/src/sgml/charset.sgml | 2 +- doc/src/sgml/config.sgml | 2 +- doc/src/sgml/func.sgml| 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-)
pgsql: Documentation improvements to jsonpath
Documentation improvements to jsonpath Besides cosmetic improvements it removes statement that operators necessary need to be separated from operands with spaces, which is not really true. Discussion: https://postgr.es/m/CAEkD-mDUZrRE%3Dk-FznEg4Ed2VdjpZCyHoyo%2Bp0%2B8KvHqR%3DpNVQ%40mail.gmail.com Author: Liudmila Mantrova, Alexander Lakhin Reviewed-by: Alexander Korotkov, Alvaro Herrera Backpatch-through: 12 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/9f771868c446c44dae6b97c68eeb1b19da32c50a Modified Files -- doc/src/sgml/func.sgml | 41 +++-- doc/src/sgml/json.sgml | 10 +- 2 files changed, 24 insertions(+), 27 deletions(-)
pgsql: Typo fixes for documentation
Typo fixes for documentation Discussion: https://postgr.es/m/CAEkD-mDUZrRE%3Dk-FznEg4Ed2VdjpZCyHoyo%2Bp0%2B8KvHqR%3DpNVQ%40mail.gmail.com Author: Liudmila Mantrova, Alexander Lakhin Reviewed-by: Alexander Korotkov, Alvaro Herrera Backpatch-through: 12 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/eb57bd9c1d83a20eaff559a53b2f584dcd0668a8 Modified Files -- doc/src/sgml/charset.sgml | 2 +- doc/src/sgml/config.sgml | 2 +- doc/src/sgml/func.sgml| 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-)
pgsql: Documentation improvements to jsonpath
Documentation improvements to jsonpath Besides cosmetic improvements it removes statement that operators necessary need to be separated from operands with spaces, which is not really true. Discussion: https://postgr.es/m/CAEkD-mDUZrRE%3Dk-FznEg4Ed2VdjpZCyHoyo%2Bp0%2B8KvHqR%3DpNVQ%40mail.gmail.com Author: Liudmila Mantrova, Alexander Lakhin Reviewed-by: Alexander Korotkov, Alvaro Herrera Backpatch-through: 12 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/c30fc9ddd596f7981555318d1c236fc55279cac8 Modified Files -- doc/src/sgml/func.sgml | 41 +++-- doc/src/sgml/json.sgml | 10 +- 2 files changed, 24 insertions(+), 27 deletions(-)
pgsql: Remove pg_trgm.strict_word_similarity_threshold doc from 9.6 and
Remove pg_trgm.strict_word_similarity_threshold doc from 9.6 and 10 9ee98cc3f added missing doc for pg_trgm.strict_word_similarity_threshold GUC. But it was accidentally backpatched to 9.6, while this GUC was introduced only in 11. This patch removes extra doc from 9.6 and 10. Discussion: https://postgr.es/m/CAHE3wghn%3DRyPWY-nxX1SiYAfkuwLHMd9Z4r8v9ww_jSs1jF5pg%40mail.gmail.com Author: Euler Taveira Backpatch-through: 9.6 and 10 Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/0a8cd4c2962404f167547e79768db79f5f710f81 Modified Files -- doc/src/sgml/pgtrgm.sgml | 17 - 1 file changed, 17 deletions(-)
pgsql: Remove pg_trgm.strict_word_similarity_threshold doc from 9.6 and
Remove pg_trgm.strict_word_similarity_threshold doc from 9.6 and 10 9ee98cc3f added missing doc for pg_trgm.strict_word_similarity_threshold GUC. But it was accidentally backpatched to 9.6, while this GUC was introduced only in 11. This patch removes extra doc from 9.6 and 10. Discussion: https://postgr.es/m/CAHE3wghn%3DRyPWY-nxX1SiYAfkuwLHMd9Z4r8v9ww_jSs1jF5pg%40mail.gmail.com Author: Euler Taveira Backpatch-through: 9.6 and 10 Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/b18aaad90fb14df8d211550f410fcb651e982ed4 Modified Files -- doc/src/sgml/pgtrgm.sgml | 17 - 1 file changed, 17 deletions(-)
pgsql: Fix handling of non-key columns get_index_column_opclass()
Fix handling of non-key columns get_index_column_opclass() f2e40380 introduces support of non-key attributes in GiST indexes. Then if get_index_column_opclass() is asked by gistproperty() to get an opclass of non-key column, it returns garbage past oidvector value. This commit fixes that by making get_index_column_opclass() return InvalidOid in this case. Discussion: https://postgr.es/m/20190902231948.GA5343%40alvherre.pgsql Author: Nikita Glukhov, Alexander Korotkov Backpatch-through: 12 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/c6ce5f71b08235f9ed8e0206267ae560ea1ebc91 Modified Files -- src/backend/utils/cache/lsyscache.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-)
pgsql: Fix handling of non-key columns get_index_column_opclass()
Fix handling of non-key columns get_index_column_opclass() f2e40380 introduces support of non-key attributes in GiST indexes. Then if get_index_column_opclass() is asked by gistproperty() to get an opclass of non-key column, it returns garbage past oidvector value. This commit fixes that by making get_index_column_opclass() return InvalidOid in this case. Discussion: https://postgr.es/m/20190902231948.GA5343%40alvherre.pgsql Author: Nikita Glukhov, Alexander Korotkov Backpatch-through: 12 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/7e04160390464cd39690d36054e0ac5e4f1bf227 Modified Files -- src/backend/utils/cache/lsyscache.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-)
pgsql: Fix handling Inf and Nan values in GiST pairing heap comparator
Fix handling Inf and Nan values in GiST pairing heap comparator Previously plain float comparison was used in GiST pairing heap. Such comparison doesn't provide proper ordering for value sets containing Inf and Nan values. This commit fixes that by usage of float8_cmp_internal(). Note, there is remaining problem with NULL distances, which are represented as Inf in pairing heap. It would be fixes in subsequent commit. Backpatch to all supported versions. Reported-by: Andrey Borodin Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Heikki Linnakangas Backpatch-through: 9.4 Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/986319d467cfefaa54b5cb72e063e28b66f04d42 Modified Files -- src/backend/access/gist/gistscan.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-)
pgsql: Fix handling Inf and Nan values in GiST pairing heap comparator
Fix handling Inf and Nan values in GiST pairing heap comparator Previously plain float comparison was used in GiST pairing heap. Such comparison doesn't provide proper ordering for value sets containing Inf and Nan values. This commit fixes that by usage of float8_cmp_internal(). Note, there is remaining problem with NULL distances, which are represented as Inf in pairing heap. It would be fixes in subsequent commit. Backpatch to all supported versions. Reported-by: Andrey Borodin Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Heikki Linnakangas Backpatch-through: 9.4 Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/b2037afec14725e4521427edbee8071d04057a2d Modified Files -- src/backend/access/gist/gistscan.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-)
pgsql: Fix handling of NULL distances in KNN-GiST
Fix handling of NULL distances in KNN-GiST In order to implement NULL LAST semantic GiST previously assumed distance to the NULL value to be Inf. However, our distance functions can return Inf and NaN for non-null values. In such cases, NULL LAST semantic appears to be broken. This commit fixes that by introducing separate array of null flags for distances. Backpatch to all supported versions. Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Backpatch-through: 9.4 Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/3c155bafa59b26e0bb38b8e70b9034183c946d39 Modified Files -- src/backend/access/gist/gistget.c | 77 +- src/backend/access/gist/gistscan.c | 25 +++-- src/include/access/gist_private.h | 26 +++-- 3 files changed, 95 insertions(+), 33 deletions(-)
pgsql: Fix handling of NULL distances in KNN-GiST
Fix handling of NULL distances in KNN-GiST In order to implement NULL LAST semantic GiST previously assumed distance to the NULL value to be Inf. However, our distance functions can return Inf and NaN for non-null values. In such cases, NULL LAST semantic appears to be broken. This commit fixes that by introducing separate array of null flags for distances. Backpatch to all supported versions. Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Backpatch-through: 9.4 Branch -- REL9_4_STABLE Details --- https://git.postgresql.org/pg/commitdiff/1df4123048c49dda878bbe3b49616b0ff3b3dbe5 Modified Files -- src/backend/access/gist/gistget.c | 70 +- src/backend/access/gist/gistscan.c | 31 + src/include/access/gist_private.h | 25 -- 3 files changed, 93 insertions(+), 33 deletions(-)
pgsql: Fix handling of NULL distances in KNN-GiST
Fix handling of NULL distances in KNN-GiST In order to implement NULL LAST semantic GiST previously assumed distance to the NULL value to be Inf. However, our distance functions can return Inf and NaN for non-null values. In such cases, NULL LAST semantic appears to be broken. This commit fixes that by introducing separate array of null flags for distances. Backpatch to all supported versions. Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Backpatch-through: 9.4 Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/d807200b4a73a100cb8f41ee2f3ff1e2507647b8 Modified Files -- src/backend/access/gist/gistget.c | 77 +- src/backend/access/gist/gistscan.c | 25 +++-- src/include/access/gist_private.h | 26 +++-- 3 files changed, 95 insertions(+), 33 deletions(-)
pgsql: Fix handling Inf and Nan values in GiST pairing heap comparator
Fix handling Inf and Nan values in GiST pairing heap comparator Previously plain float comparison was used in GiST pairing heap. Such comparison doesn't provide proper ordering for value sets containing Inf and Nan values. This commit fixes that by usage of float8_cmp_internal(). Note, there is remaining problem with NULL distances, which are represented as Inf in pairing heap. It would be fixes in subsequent commit. Backpatch to all supported versions. Reported-by: Andrey Borodin Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Heikki Linnakangas Backpatch-through: 9.4 Branch -- REL9_4_STABLE Details --- https://git.postgresql.org/pg/commitdiff/111fb7e42e335bbe104939418ec44c903d3b2329 Modified Files -- src/backend/access/gist/gistscan.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-)
pgsql: Fix handling of NULL distances in KNN-GiST
Fix handling of NULL distances in KNN-GiST In order to implement NULL LAST semantic GiST previously assumed distance to the NULL value to be Inf. However, our distance functions can return Inf and NaN for non-null values. In such cases, NULL LAST semantic appears to be broken. This commit fixes that by introducing separate array of null flags for distances. Backpatch to all supported versions. Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Backpatch-through: 9.4 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/02f90879e75b3d4ccdba1ec7c3cad6af08dff77d Modified Files -- src/backend/access/gist/gistget.c | 70 -- src/backend/access/gist/gistscan.c | 25 +-- src/backend/access/index/indexam.c | 14 -- src/backend/access/spgist/spgscan.c| 1 + src/include/access/genam.h | 4 +- src/include/access/gist_private.h | 26 +-- src/test/regress/expected/create_index.out | 2 +- 7 files changed, 106 insertions(+), 36 deletions(-)
pgsql: Fix handling of NULL distances in KNN-GiST
Fix handling of NULL distances in KNN-GiST In order to implement NULL LAST semantic GiST previously assumed distance to the NULL value to be Inf. However, our distance functions can return Inf and NaN for non-null values. In such cases, NULL LAST semantic appears to be broken. This commit fixes that by introducing separate array of null flags for distances. Backpatch to all supported versions. Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Backpatch-through: 9.4 Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/92f6b49c48a3b9c829d4117d59e89128455dbf5f Modified Files -- src/backend/access/gist/gistget.c | 77 +- src/backend/access/gist/gistscan.c | 25 +++-- src/include/access/gist_private.h | 26 +++-- 3 files changed, 95 insertions(+), 33 deletions(-)
pgsql: Fix handling Inf and Nan values in GiST pairing heap comparator
Fix handling Inf and Nan values in GiST pairing heap comparator Previously plain float comparison was used in GiST pairing heap. Such comparison doesn't provide proper ordering for value sets containing Inf and Nan values. This commit fixes that by usage of float8_cmp_internal(). Note, there is remaining problem with NULL distances, which are represented as Inf in pairing heap. It would be fixes in subsequent commit. Backpatch to all supported versions. Reported-by: Andrey Borodin Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Heikki Linnakangas Backpatch-through: 9.4 Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/749b04d1d8b8ad63d365b8f6ad9d70843f3c1239 Modified Files -- src/backend/access/gist/gistscan.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-)
pgsql: Fix handling Inf and Nan values in GiST pairing heap comparator
Fix handling Inf and Nan values in GiST pairing heap comparator Previously plain float comparison was used in GiST pairing heap. Such comparison doesn't provide proper ordering for value sets containing Inf and Nan values. This commit fixes that by usage of float8_cmp_internal(). Note, there is remaining problem with NULL distances, which are represented as Inf in pairing heap. It would be fixes in subsequent commit. Backpatch to all supported versions. Reported-by: Andrey Borodin Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Heikki Linnakangas Backpatch-through: 9.4 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/e5d8f3596100da0d38a38513c69e803b7fe7041a Modified Files -- src/backend/access/gist/gistscan.c | 7 +-- src/test/regress/expected/create_index.out | 10 +- 2 files changed, 10 insertions(+), 7 deletions(-)
pgsql: Fix handling Inf and Nan values in GiST pairing heap comparator
Fix handling Inf and Nan values in GiST pairing heap comparator Previously plain float comparison was used in GiST pairing heap. Such comparison doesn't provide proper ordering for value sets containing Inf and Nan values. This commit fixes that by usage of float8_cmp_internal(). Note, there is remaining problem with NULL distances, which are represented as Inf in pairing heap. It would be fixes in subsequent commit. Backpatch to all supported versions. Reported-by: Andrey Borodin Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Heikki Linnakangas Backpatch-through: 9.4 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/bc67f4189a7a3381db76dbfeafca463187bfe87a Modified Files -- src/backend/access/gist/gistscan.c | 7 +-- src/test/regress/expected/create_index.out | 10 +- 2 files changed, 10 insertions(+), 7 deletions(-)
pgsql: Fix handling Inf and Nan values in GiST pairing heap comparator
Fix handling Inf and Nan values in GiST pairing heap comparator Previously plain float comparison was used in GiST pairing heap. Such comparison doesn't provide proper ordering for value sets containing Inf and Nan values. This commit fixes that by usage of float8_cmp_internal(). Note, there is remaining problem with NULL distances, which are represented as Inf in pairing heap. It would be fixes in subsequent commit. Backpatch to all supported versions. Reported-by: Andrey Borodin Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Heikki Linnakangas Backpatch-through: 9.4 Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/8f724002e2fe0e415f5a603462440d2f297f287c Modified Files -- src/backend/access/gist/gistscan.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-)
pgsql: Fix handling of NULL distances in KNN-GiST
Fix handling of NULL distances in KNN-GiST In order to implement NULL LAST semantic GiST previously assumed distance to the NULL value to be Inf. However, our distance functions can return Inf and NaN for non-null values. In such cases, NULL LAST semantic appears to be broken. This commit fixes that by introducing separate array of null flags for distances. Backpatch to all supported versions. Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Backpatch-through: 9.4 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/e6af7b367cf46fc385307efcef5a1fece0e5fef3 Modified Files -- src/backend/access/gist/gistget.c | 70 -- src/backend/access/gist/gistscan.c | 25 +-- src/backend/access/index/indexam.c | 14 -- src/backend/access/spgist/spgscan.c| 1 + src/include/access/genam.h | 4 +- src/include/access/gist_private.h | 26 +-- src/test/regress/expected/create_index.out | 2 +- 7 files changed, 106 insertions(+), 36 deletions(-)
pgsql: Fix handling of NULL distances in KNN-GiST
Fix handling of NULL distances in KNN-GiST In order to implement NULL LAST semantic GiST previously assumed distance to the NULL value to be Inf. However, our distance functions can return Inf and NaN for non-null values. In such cases, NULL LAST semantic appears to be broken. This commit fixes that by introducing separate array of null flags for distances. Backpatch to all supported versions. Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Backpatch-through: 9.4 Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/a5431b7d5f6bcccd1fc69c4e4ae6e4d5f39b83f8 Modified Files -- src/backend/access/gist/gistget.c | 77 +- src/backend/access/gist/gistscan.c | 25 +++-- src/include/access/gist_private.h | 26 +++-- 3 files changed, 95 insertions(+), 33 deletions(-)
Re: pgsql: Adjust string comparison in jsonpath
пн, 12 авг. 2019 г., 3:25 Thomas Munro : > On Mon, Aug 12, 2019 at 10:30 AM Alexander Korotkov > wrote: > > > > This appears to have upset prion when testing on en_US.iso885915. > > > > > > Also lapwing's "InstallCheck-fr_FR" stage crashed on this commit, when > > > running JSON queries, on HEAD and REL_12_STABLE: > > > Thank you for pointing! I hope I can investigate this shortly. > > Hi Alexander, > > I can reproduce this by running LANG="fr_FR.ISO8859-1" initdb, then > running installcheck (on some other OSes that might be called just > "fr_FR"). See this comment in mbutils.c: > > * The functions return a palloc'd, null-terminated string if conversion > * is required. However, if no conversion is performed, the given source > * string pointer is returned as-is. > > You call pfree() on the result of pg_server_to_any() without checking > if it just returned in the input pointer (for example, it does that if > you give it an empty string). That triggers an assertion failure > somewhere inside pfree(). The following fixes that for me, and is > based on code I found elsewhere in the tree. > > --- a/src/backend/utils/adt/jsonpath_exec.c > +++ b/src/backend/utils/adt/jsonpath_exec.c > @@ -2028,8 +2028,10 @@ compareStrings(const char *mbstr1, int mblen1, > cmp = binaryCompareStrings(utf8str1, strlen(utf8str1), > > utf8str2, strlen(utf8str2)); > > - pfree(utf8str1); > - pfree(utf8str2); > + if (mbstr1 != utf8str1) > + pfree(utf8str1); > + if (mbstr2 != utf8str2) > + pfree(utf8str2); > > With that fixed it no longer crashes, but then the regression test > fails due to differences in the output, which look like locale > ordering differences. > Thank you for the diagnostics. Should be fixed by 251c8e39. BTW, test failures appears to be caused not by locale differences, but by using strlen() on non null-terminated original strings. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company >
pgsql: Fix string comparison in jsonpath
Fix string comparison in jsonpath Take into account pg_server_to_any() may return input string "as is". Reported-by: Andrew Dunstan, Thomas Munro Discussion: https://postgr.es/m/0ed83a33-d900-466a-880a-70ef456c721f%402ndQuadrant.com Author: Alexander Korotkov, Thomas Munro Backpatch-through: 12 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/251c8e39bc6b0a3ff1620d9ac10888a7660e6b88 Modified Files -- src/backend/utils/adt/jsonpath_exec.c | 37 +++ 1 file changed, 29 insertions(+), 8 deletions(-)
Re: pgsql: Adjust string comparison in jsonpath
On Mon, Aug 12, 2019 at 1:25 AM Thomas Munro wrote: > On Mon, Aug 12, 2019 at 9:04 AM Andrew Dunstan > wrote: > > On 8/11/19 4:10 PM, Alexander Korotkov wrote: > > > Adjust string comparison in jsonpath > > > > > > We have implemented jsonpath string comparison using default database > > > locale. > > > However, standard requires us to compare Unicode codepoints. This commit > > > implements that, but for performance reasons we still use per-byte > > > comparison > > > for "==" operator. Thus, for consistency other comparison operators do > > > per-byte > > > comparison if Unicode codepoints appear to be equal. > > > > > > In some edge cases, when same Unicode codepoints have different binary > > > representations in database encoding, we diverge standard to achieve > > > better > > > performance of "==" operator. In future to implement strict standard > > > conformance, we can do normalization of input JSON strings. > > > > > > > > > This appears to have upset prion when testing on en_US.iso885915. > > Also lapwing's "InstallCheck-fr_FR" stage crashed on this commit, when > running JSON queries, on HEAD and REL_12_STABLE: > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing=2019-08-11%2021%3A02%3A32 > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing=2019-08-11%2020%3A40%3A07 Thank you for pointing! I hope I can investigate this shortly. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
pgsql: Adjust string comparison in jsonpath
Adjust string comparison in jsonpath We have implemented jsonpath string comparison using default database locale. However, standard requires us to compare Unicode codepoints. This commit implements that, but for performance reasons we still use per-byte comparison for "==" operator. Thus, for consistency other comparison operators do per-byte comparison if Unicode codepoints appear to be equal. In some edge cases, when same Unicode codepoints have different binary representations in database encoding, we diverge standard to achieve better performance of "==" operator. In future to implement strict standard conformance, we can do normalization of input JSON strings. Original patch was written by Nikita Glukhov, rewritten by me. Reported-by: Markus Winand Discussion: https://postgr.es/m/8B7FA3B4-328D-43D7-95A8-37B8891B8C78%40winand.at Author: Nikita Glukhov, Alexander Korotkov Backpatch-through: 12 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/d54ceb9e176152f930e60709e07c636e8e5414f5 Modified Files -- src/backend/utils/adt/jsonpath_exec.c| 72 +++- src/test/regress/expected/jsonb_jsonpath.out | 163 +++ src/test/regress/sql/jsonb_jsonpath.sql | 16 +++ 3 files changed, 248 insertions(+), 3 deletions(-)
pgsql: Adjust string comparison in jsonpath
Adjust string comparison in jsonpath We have implemented jsonpath string comparison using default database locale. However, standard requires us to compare Unicode codepoints. This commit implements that, but for performance reasons we still use per-byte comparison for "==" operator. Thus, for consistency other comparison operators do per-byte comparison if Unicode codepoints appear to be equal. In some edge cases, when same Unicode codepoints have different binary representations in database encoding, we diverge standard to achieve better performance of "==" operator. In future to implement strict standard conformance, we can do normalization of input JSON strings. Original patch was written by Nikita Glukhov, rewritten by me. Reported-by: Markus Winand Discussion: https://postgr.es/m/8B7FA3B4-328D-43D7-95A8-37B8891B8C78%40winand.at Author: Nikita Glukhov, Alexander Korotkov Backpatch-through: 12 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/3218ff5c6aea5841ab547ecca26927716419fe4b Modified Files -- src/backend/utils/adt/jsonpath_exec.c| 72 +++- src/test/regress/expected/jsonb_jsonpath.out | 163 +++ src/test/regress/sql/jsonb_jsonpath.sql | 16 +++ 3 files changed, 248 insertions(+), 3 deletions(-)
pgsql: Fix some typos in jsonpath documentation
Fix some typos in jsonpath documentation Discussion: https://postgr.es/m/8B7FA3B4-328D-43D7-95A8-37B8891B8C78%40winand.at Author: Markus Winand Backpatch-through: 12 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/53d467246589de631ce1846105f990099219e737 Modified Files -- doc/src/sgml/func.sgml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Fix some typos in jsonpath documentation
Fix some typos in jsonpath documentation Discussion: https://postgr.es/m/8B7FA3B4-328D-43D7-95A8-37B8891B8C78%40winand.at Author: Markus Winand Backpatch-through: 12 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/1f33f211bc531d257f84fefb9208dd43e8718972 Modified Files -- doc/src/sgml/func.sgml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Re: pgsql: Forgotten catversion bump
On Sun, Jul 14, 2019 at 3:28 PM Alexander Korotkov wrote: > 6254c55f81, c085e1c1cb and 075f0a880f all change system catalog. But > catversion bump is missed in all of them. So, do catversion bump now. > > Also, I need mention patch reviewer Fabien Coelho, who has been missed in > commit messages of 6254c55f81, c085e1c1cb and 075f0a880f. Sorry for both these oversights... -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
pgsql: Forgotten catversion bump
Forgotten catversion bump 6254c55f81, c085e1c1cb and 075f0a880f all change system catalog. But catversion bump is missed in all of them. So, do catversion bump now. Also, I need mention patch reviewer Fabien Coelho, who has been missed in commit messages of 6254c55f81, c085e1c1cb and 075f0a880f. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/565f3390005318ea4c982b8d054d56e9fe5a6454 Modified Files -- src/include/catalog/catversion.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Add support for <-> (box, point) operator to SP-GiST box_ops
Add support for <-> (box, point) operator to SP-GiST box_ops Opclass support functions already can handle this operator, just catalog adjustment appears to be required. Discussion: https://postgr.es/m/f71ba19d-d989-63b6-f04a-abf02ad9345d%40postgrespro.ru Author: Nikita Glukhov Reviewed-by: Tom Lane, Alexander Korotkov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/075f0a880fbf790f4a3dcac83e2d5125bcad9646 Modified Files -- doc/src/sgml/spgist.sgml | 1 + src/include/catalog/pg_amop.dat| 4 ++ src/test/regress/expected/box.out | 82 +- src/test/regress/expected/sanity_check.out | 2 + src/test/regress/sql/box.sql | 70 - 5 files changed, 135 insertions(+), 24 deletions(-)
pgsql: Add missing commutators for distance operators
Add missing commutators for distance operators Some of <-> operators between geometric types have their commutators missed. This commit adds them. The motivation is upcoming kNN support for some of those operators. Discussion: https://postgr.es/m/f71ba19d-d989-63b6-f04a-abf02ad9345d%40postgrespro.ru Author: Nikita Glukhov Reviewed-by: Tom Lane, Alexander Korotkov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/6254c55f815623bb74e2cf27562437dc3b2aa2c8 Modified Files -- src/backend/utils/adt/geo_ops.c| 136 - src/include/catalog/pg_operator.dat| 42 +- src/include/catalog/pg_proc.dat| 25 + src/test/regress/expected/geometry.out | 986 + src/test/regress/sql/geometry.sql | 15 +- 5 files changed, 687 insertions(+), 517 deletions(-)
pgsql: Add support for <-> (box, point) operator to GiST box_ops
Add support for <-> (box, point) operator to GiST box_ops Index-based calculation of this operator is exact. So, signature of gist_bbox_distance() function is changes so that caller is responsible for setting *recheck flag. Discussion: https://postgr.es/m/f71ba19d-d989-63b6-f04a-abf02ad9345d%40postgrespro.ru Author: Nikita Glukhov Reviewed-by: Tom Lane, Alexander Korotkov Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/c085e1c1cb4e29637552f5d250d45ad0cb83e5cf Modified Files -- doc/src/sgml/gist.sgml | 1 + src/backend/access/gist/gistproc.c | 48 +++- src/include/catalog/pg_amop.dat| 3 ++ src/include/catalog/pg_amproc.dat | 2 + src/include/catalog/pg_proc.dat| 4 ++ src/test/regress/expected/gist.out | 76 ++ src/test/regress/sql/gist.sql | 16 7 files changed, 133 insertions(+), 17 deletions(-)
pgsql: Fixes for jsonpath filter expression elements table in docs
Fixes for jsonpath filter expression elements table in docs Reported-by: Thom Brown Discussion: https://postgr.es/m/CAA-aLv4Tggy6Z3kaG9n%2B3SHwOVGN2Yj_MJXfdfwjH_jBNZzJNA%40mail.gmail.com Backpatch-through: 12 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/1ed89826b82c693c311d9974f3f3138013f1b929 Modified Files -- doc/src/sgml/func.sgml | 8 1 file changed, 4 insertions(+), 4 deletions(-)
pgsql: Fixes for jsonpath filter expression elements table in docs
Fixes for jsonpath filter expression elements table in docs Reported-by: Thom Brown Discussion: https://postgr.es/m/CAA-aLv4Tggy6Z3kaG9n%2B3SHwOVGN2Yj_MJXfdfwjH_jBNZzJNA%40mail.gmail.com Backpatch-through: 12 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/0cea6eb5a5f2948c411706cabfde32ce61df0d7a Modified Files -- doc/src/sgml/func.sgml | 8 1 file changed, 4 insertions(+), 4 deletions(-)
