public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 03/23] cirrus/ccache: disable compression and show stats
14+ messages / 5 participants
[nested] [flat]
* [PATCH 03/23] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)
Since v4.0, ccache enables zstd compression by default, saving roughly
2x-3x. But, cirrus caches are compressed as tar.gz, so we could disable
ccache compression, allowing cirrus to gzip the uncompressed data
(better than ccache's default of zstd-1).
With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte 109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte 52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte 134064
windows has 4.6.1; 180MB cirrus cache; cache_size_kibibyte 51179
todo: compiler warnings
With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte 316136
macos: 41MB cirrus cache; cache_size_kibibyte 118068
windows: 42MB cirrus cache; cache_size_kibibyte 134064
freebsd is the same
The stats should either be shown or logged (or maybe run with CCACHE_NOSTATS,
to avoid re-uploading cache tarball in a 100% cached build, due only to
updating ./**/stats).
Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.
freebsd, linux
ci-os-only: macos
---
.cirrus.yml | 33 ++++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 591e802bee2..54256570030 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,9 @@ env:
# Useful to be able to analyse what in a script takes long
CIRRUS_LOG_TIMESTAMP: true
- CCACHE_MAXSIZE: "250M"
+ CCACHE_MAXSIZE: "500M"
+ CCACHE_NOCOMPRESS: 1
+ #CCACHE_STATSLOG: ccache-stats.log
# target to test, for all but windows
CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -110,7 +112,10 @@ task:
CXX="ccache c++" \
CFLAGS="-Og -ggdb"
EOF
- build_script: su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+ build_script: |
+ su postgres -c "ccache --zero-stats"
+ su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+ su postgres -c "ccache --show-stats"
upload_caches: ccache
# The use of script avoids make -Otarget complaints about fcntl() on
@@ -206,7 +211,11 @@ task:
CFLAGS="-Og -ggdb" \
CXXFLAGS="-Og -ggdb"
EOF
- build_script: su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+
+ build_script: |
+ su postgres -c "ccache --zero-stats"
+ su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+ su postgres -c "ccache --show-stats"
upload_caches: ccache
test_world_script: |
@@ -324,7 +333,11 @@ task:
\
LLVM_CONFIG=${brewpath}/llvm/bin/llvm-config \
PYTHON=python3
- build_script: gmake -s -j${BUILD_JOBS} world-bin
+
+ build_script: |
+ ccache --zero-stats
+ gmake -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats -v
upload_caches: ccache
test_world_script: |
@@ -482,7 +495,7 @@ task:
# Use larger ccache cache, as this task compiles with multiple compilers /
# flag combinations
- CCACHE_MAXSIZE: "1GB"
+ CCACHE_MAXSIZE: "1500MB"
CCACHE_DIR: "/tmp/ccache_dir"
LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
@@ -530,7 +543,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# gcc, cassert on, dtrace off
always:
@@ -541,7 +556,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# clang, cassert off, dtrace off
always:
@@ -551,7 +568,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# clang, cassert on, dtrace on
always:
@@ -563,7 +582,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# cross-compile to windows
always:
@@ -574,7 +595,9 @@ task:
CC="ccache x86_64-w64-mingw32-gcc" \
CXX="ccache x86_64-w64-mingw32-g++"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
###
# Verify docs can be built
--
2.17.1
--tKkaNMvYmhQvRCRK
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0004-cirrus-ccache-add-explicit-cache-keys.patch"
^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH 03/23] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)
Since v4.0, ccache enables zstd compression by default, saving roughly
2x-3x. But, cirrus caches are compressed as tar.gz, so we could disable
ccache compression, allowing cirrus to gzip the uncompressed data
(better than ccache's default of zstd-1).
With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte 109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte 52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte 134064
windows has 4.6.1; 180MB cirrus cache; cache_size_kibibyte 51179
todo: compiler warnings
With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte 316136
macos: 41MB cirrus cache; cache_size_kibibyte 118068
windows: 42MB cirrus cache; cache_size_kibibyte 134064
freebsd is the same
The stats should either be shown or logged (or maybe run with CCACHE_NOSTATS,
to avoid re-uploading cache tarball in a 100% cached build, due only to
updating ./**/stats).
Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.
freebsd, linux
ci-os-only: macos
---
.cirrus.yml | 33 ++++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 591e802bee2..54256570030 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,9 @@ env:
# Useful to be able to analyse what in a script takes long
CIRRUS_LOG_TIMESTAMP: true
- CCACHE_MAXSIZE: "250M"
+ CCACHE_MAXSIZE: "500M"
+ CCACHE_NOCOMPRESS: 1
+ #CCACHE_STATSLOG: ccache-stats.log
# target to test, for all but windows
CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -110,7 +112,10 @@ task:
CXX="ccache c++" \
CFLAGS="-Og -ggdb"
EOF
- build_script: su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+ build_script: |
+ su postgres -c "ccache --zero-stats"
+ su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+ su postgres -c "ccache --show-stats"
upload_caches: ccache
# The use of script avoids make -Otarget complaints about fcntl() on
@@ -206,7 +211,11 @@ task:
CFLAGS="-Og -ggdb" \
CXXFLAGS="-Og -ggdb"
EOF
- build_script: su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+
+ build_script: |
+ su postgres -c "ccache --zero-stats"
+ su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+ su postgres -c "ccache --show-stats"
upload_caches: ccache
test_world_script: |
@@ -324,7 +333,11 @@ task:
\
LLVM_CONFIG=${brewpath}/llvm/bin/llvm-config \
PYTHON=python3
- build_script: gmake -s -j${BUILD_JOBS} world-bin
+
+ build_script: |
+ ccache --zero-stats
+ gmake -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats -v
upload_caches: ccache
test_world_script: |
@@ -482,7 +495,7 @@ task:
# Use larger ccache cache, as this task compiles with multiple compilers /
# flag combinations
- CCACHE_MAXSIZE: "1GB"
+ CCACHE_MAXSIZE: "1500MB"
CCACHE_DIR: "/tmp/ccache_dir"
LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
@@ -530,7 +543,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# gcc, cassert on, dtrace off
always:
@@ -541,7 +556,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# clang, cassert off, dtrace off
always:
@@ -551,7 +568,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# clang, cassert on, dtrace on
always:
@@ -563,7 +582,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# cross-compile to windows
always:
@@ -574,7 +595,9 @@ task:
CC="ccache x86_64-w64-mingw32-gcc" \
CXX="ccache x86_64-w64-mingw32-g++"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
###
# Verify docs can be built
--
2.17.1
--tKkaNMvYmhQvRCRK
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0004-cirrus-ccache-add-explicit-cache-keys.patch"
^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH 13/21] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)
ci-os-only: linux, windows
ccache since 4.0 enables zstd compression by default.
With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte 109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte 52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte 134064
windows has 4.6.1; 180MB cirrus cache; cache_size_kibibyte 51179
todo: compiler warnings
With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte 316136
macos: 41MB cirrus cache; cache_size_kibibyte 118068
windows: 42MB cirrus cache; cache_size_kibibyte 134064
freebsd is the same
The stats should either be shown or logged (or maybe run with CCACHE_NOSTATS,
to avoid re-uploading cache tarball in a 100% cached build, due only to
updating ./**/stats).
Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.
---
.cirrus.yml | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 50a34373647..1d16594803f 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,9 @@ env:
# Useful to be able to analyse what in a script takes long
CIRRUS_LOG_TIMESTAMP: true
- CCACHE_MAXSIZE: "250M"
+ CCACHE_MAXSIZE: "500M"
+ CCACHE_NOCOMPRESS: 1
+ #CCACHE_STATSLOG: ccache.log
# target to test, for all but windows
CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -110,7 +112,11 @@ task:
CXX="ccache c++" \
CFLAGS="-Og -ggdb"
EOF
- build_script: su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+
+ build_script: |
+ ccache --zero-stats
+ su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+ ccache --show-stats
upload_caches: ccache
# The use of script avoids make -Otarget complaints about fcntl() on
@@ -206,7 +212,11 @@ task:
CFLAGS="-Og -ggdb -fsanitize=undefined,alignment -fno-sanitize-recover=all" \
CXXFLAGS="-Og -ggdb -fsanitize=undefined,alignment -fno-sanitize-recover=all"
EOF
- build_script: su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+
+ build_script: |
+ ccache --zero-stats
+ su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+ ccache --show-stats
upload_caches: ccache
test_world_script: |
@@ -325,7 +335,11 @@ task:
\
LLVM_CONFIG=${brewpath}/llvm/bin/llvm-config \
PYTHON=python3
- build_script: gmake -s -j${BUILD_JOBS} world-bin
+
+ build_script: |
+ ccache --zero-stats
+ gmake -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
upload_caches: ccache
test_world_script: |
@@ -422,7 +436,9 @@ task:
build_script:
#- ccache --clear
- vcvarsall x64
+ - ccache --zero-stats
- msbuild %MSBFLAGS% pgsql.sln
+ - ccache --show-stats
always:
upload_caches: ccache
@@ -537,7 +553,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# gcc, cassert on, dtrace off
always:
@@ -548,7 +566,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# clang, cassert off, dtrace off
always:
@@ -558,7 +578,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# clang, cassert on, dtrace on
always:
@@ -570,7 +592,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# cross-compile to windows
always:
@@ -581,7 +605,9 @@ task:
CC="ccache x86_64-w64-mingw32-gcc" \
CXX="ccache x86_64-w64-mingw32-g++"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
###
# Verify docs can be built
--
2.17.1
--mln0rGgUGuXEqmuI
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0014-another-way-to-install-uri-regress-uri_regress.patch"
^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH 03/25] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)
ccache since 4.0 enables zstd compression by default.
With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte 109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte 52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte 134064
windows has 4.6.1; 180MB cirrus cache; cache_size_kibibyte 51179
todo: compiler warnings
With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte 316136
macos: 41MB cirrus cache; cache_size_kibibyte 118068
windows: 42MB cirrus cache; cache_size_kibibyte 134064
freebsd is the same
The stats should either be shown or logged (or maybe run with CCACHE_NOSTATS,
to avoid re-uploading cache tarball in a 100% cached build, due only to
updating ./**/stats).
Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.
freebsd, linux
ci-os-only: macos
---
.cirrus.yml | 33 ++++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index f2861176be2..c0616e8560d 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,9 @@ env:
# Useful to be able to analyse what in a script takes long
CIRRUS_LOG_TIMESTAMP: true
- CCACHE_MAXSIZE: "250M"
+ CCACHE_MAXSIZE: "500M"
+ CCACHE_NOCOMPRESS: 1
+ #CCACHE_STATSLOG: ccache-stats.log
# target to test, for all but windows
CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -110,7 +112,10 @@ task:
CXX="ccache c++" \
CFLAGS="-Og -ggdb"
EOF
- build_script: su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+ build_script: |
+ su postgres -c "ccache --zero-stats"
+ su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+ su postgres -c "ccache --show-stats"
upload_caches: ccache
# The use of script avoids make -Otarget complaints about fcntl() on
@@ -206,7 +211,11 @@ task:
CFLAGS="-Og -ggdb" \
CXXFLAGS="-Og -ggdb"
EOF
- build_script: su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+
+ build_script: |
+ su postgres -c "ccache --zero-stats"
+ su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+ su postgres -c "ccache --show-stats"
upload_caches: ccache
test_world_script: |
@@ -324,7 +333,11 @@ task:
\
LLVM_CONFIG=${brewpath}/llvm/bin/llvm-config \
PYTHON=python3
- build_script: gmake -s -j${BUILD_JOBS} world-bin
+
+ build_script: |
+ ccache --zero-stats
+ gmake -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats -v
upload_caches: ccache
test_world_script: |
@@ -476,7 +489,7 @@ task:
# Use larger ccache cache, as this task compiles with multiple compilers /
# flag combinations
- CCACHE_MAXSIZE: "1GB"
+ CCACHE_MAXSIZE: "1500MB"
CCACHE_DIR: "/tmp/ccache_dir"
LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
@@ -524,7 +537,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# gcc, cassert on, dtrace off
always:
@@ -535,7 +550,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# clang, cassert off, dtrace off
always:
@@ -545,7 +562,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# clang, cassert on, dtrace on
always:
@@ -557,7 +576,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# cross-compile to windows
always:
@@ -568,7 +589,9 @@ task:
CC="ccache x86_64-w64-mingw32-gcc" \
CXX="ccache x86_64-w64-mingw32-g++"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
###
# Verify docs can be built
--
2.17.1
--IS0zKkzwUGydFO0o
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0004-cirrus-ccache-add-explicit-cache-keys.patch"
^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH 7/8] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)
Since v4.0, ccache enables zstd compression by default, saving roughly
2x-3x. But, cirrus caches are compressed as tar.gz, so we could disable
ccache compression, allowing cirrus to gzip the uncompressed data
(better than ccache's default of zstd-1).
With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte 109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte 52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte 134064
XXX windows has 4.7.2; 180MB cirrus cache; cache_size_kibibyte 51179
todo: compiler warnings
With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte 316136
macos: 41MB cirrus cache; cache_size_kibibyte 118068
windows: 42MB cirrus cache; cache_size_kibibyte 134064
freebsd is the same
The stats should be shown and/or logged.
ccache --show-stats shows the *cumulative* stats (including prior
compilations)
ccache --zero-stats clears out not only the global stats, but the
per-file cache stats (from which the global stats are derived) - which
obviously makes the cache work poorly.
Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.
The log should be written *outside* the ccache folder - it shouldn't be
preserved across cirrusci task invocations.
freebsd, linux, macos
ci-os-only: linux
---
.cirrus.yml | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 221431e70c4..d1c3119e4ff 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,9 @@ env:
# Useful to be able to analyse what in a script takes long
CIRRUS_LOG_TIMESTAMP: true
- CCACHE_MAXSIZE: "250M"
+ CCACHE_MAXSIZE: "750M"
+ CCACHE_NOCOMPRESS: 1
+ #CCACHE_STATSLOG: ccache-stats.log
# target to test, for all but windows
CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -236,7 +238,7 @@ task:
- name: Linux - Debian Bullseye - Meson
env:
- CCACHE_MAXSIZE: "400M" # tests two different builds
+ CCACHE_MAXSIZE: "1200M" # tests two different builds
configure_script: |
su postgres <<-EOF
@@ -307,6 +309,7 @@ task:
CIRRUS_WORKING_DIR: ${HOME}/pgsql/
CCACHE_DIR: ${HOME}/ccache
+ CCACHE_STATSLOG: ${CCACHE_DIR}.stats.log
HOMEBREW_CACHE: ${HOME}/homebrew-cache
PERL5LIB: ${HOME}/perl5/lib/perl5
@@ -388,7 +391,7 @@ task:
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
build
- build_script: ninja -C build -j${BUILD_JOBS}
+ build_script: ninja -C build -j${BUILD_JOBS} && ccache --show-log-stats
upload_caches: ccache
test_world_script: |
@@ -496,14 +499,14 @@ task:
# Use larger ccache cache, as this task compiles with multiple compilers /
# flag combinations
- CCACHE_MAXSIZE: "1GB"
+ CCACHE_MAXSIZE: "3G"
CCACHE_DIR: "/tmp/ccache_dir"
LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
LINUX_MESON_FEATURES: *LINUX_MESON_FEATURES
# task that did not run, count as a success, so we need to recheck Linux'
- # condition here ...
+ # condition here; cirus warns if the "only_if" condition doesn't match the task being depended on
only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*linux.*'
container:
--
2.25.1
--rPFbv4B2w2tcwGo5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0008-cirrus-warnings-use-a-single-common-always-block.patch"
^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH 05/11] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)
Since v4.0, ccache enables zstd compression by default, saving roughly
2x-3x. But, cirrus caches are compressed as tar.gz, so we could disable
ccache compression, allowing cirrus to gzip the uncompressed data
(better than ccache's default of zstd-1).
With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte 109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte 52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte 134064
XXX windows has 4.7.2; 180MB cirrus cache; cache_size_kibibyte 51179
todo: compiler warnings
With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte 316136
macos: 41MB cirrus cache; cache_size_kibibyte 118068
windows: 42MB cirrus cache; cache_size_kibibyte 134064
freebsd is the same
The stats should be shown and/or logged.
ccache --show-stats shows the *cumulative* stats (including prior
compilations)
ccache --zero-stats clears out not only the global stats, but the
per-file cache stats (from which the global stats are derived) - which
obviously makes the cache work poorly.
Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.
The log should be written *outside* the ccache folder - it shouldn't be
preserved across cirrusci task invocations.
freebsd, linux, macos
ci-os-only: linux
---
.cirrus.yml | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 47c4caf51ac..d9660149a73 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,11 @@ env:
# Useful to be able to analyse what in a script takes long
CIRRUS_LOG_TIMESTAMP: true
- CCACHE_MAXSIZE: "250M"
+ CCACHE_MAXSIZE: "750M"
+ CCACHE_HARDLINK: 1
+ #CCACHE_NOCOMPRESS: 1
+ # The statslog should be stored somewhere *outside* the persistent ccache dir
+ CCACHE_STATSLOG: ${CIRRUS_WORKING_DIR}/ccache-stats.log
# target to test, for all but windows
CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -236,7 +240,7 @@ task:
- name: Linux - Debian Bullseye - Meson
env:
- CCACHE_MAXSIZE: "800M" # tests two different builds
+ CCACHE_MAXSIZE: "2400M" # tests two different builds
configure_script: |
su postgres <<-EOF
@@ -289,7 +293,7 @@ task:
ccache_stats_end_script:
ccache -s
- CCACHE_MAXSIZE=400M ccache --cleanup
+ CCACHE_MAXSIZE=1200M ccache --cleanup
on_failure:
<<: *on_failure_meson
@@ -392,7 +396,7 @@ task:
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
build
- build_script: ninja -C build -j${BUILD_JOBS}
+ build_script: ninja -C build -j${BUILD_JOBS} && ccache --show-log-stats
upload_caches: ccache
test_world_script: |
@@ -499,7 +503,7 @@ task:
# flag combinations. The cache size is set high now, and manually pruned later,
# to avoid cache cleaning cycles during the early compilations pruning
# compilation results which could've been used during later compilations.
- CCACHE_MAXSIZE: "2G"
+ CCACHE_MAXSIZE: "6G"
CCACHE_DIR: "/tmp/ccache_dir"
LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
@@ -620,7 +624,7 @@ task:
always:
ccache_stats_end_script:
ccache -s
- CCACHE_MAXSIZE=1G ccache --cleanup
+ CCACHE_MAXSIZE=3G ccache --cleanup
upload_caches: ccache
--
2.25.1
--I4VOKWutKNZEOIPu
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0006-cirrus-warnings-use-a-single-common-always-block.patch"
^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH 13/21] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)
ci-os-only: linux, windows
ccache since 4.0 enables zstd compression by default.
With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte 109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte 52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte 134064
windows has 4.6.1; 180MB cirrus cache; cache_size_kibibyte 51179
todo: compiler warnings
With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte 316136
macos: 41MB cirrus cache; cache_size_kibibyte 118068
windows: 42MB cirrus cache; cache_size_kibibyte 134064
freebsd is the same
The stats should either be shown or logged (or maybe run with CCACHE_NOSTATS,
to avoid re-uploading cache tarball in a 100% cached build, due only to
updating ./**/stats).
Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.
---
.cirrus.yml | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index e5654bd5589..fd8f27ff593 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,9 @@ env:
# Useful to be able to analyse what in a script takes long
CIRRUS_LOG_TIMESTAMP: true
- CCACHE_MAXSIZE: "250M"
+ CCACHE_MAXSIZE: "500M"
+ CCACHE_NOCOMPRESS: 1
+ #CCACHE_STATSLOG: ccache.log
# target to test, for all but windows
CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -110,7 +112,11 @@ task:
CXX="ccache c++" \
CFLAGS="-Og -ggdb"
EOF
- build_script: su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+
+ build_script: |
+ ccache --zero-stats
+ su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+ ccache --show-stats
upload_caches: ccache
# The use of script avoids make -Otarget complaints about fcntl() on
@@ -206,7 +212,11 @@ task:
CFLAGS="-Og -ggdb -fsanitize=undefined,alignment -fno-sanitize-recover=all" \
CXXFLAGS="-Og -ggdb -fsanitize=undefined,alignment -fno-sanitize-recover=all"
EOF
- build_script: su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+
+ build_script: |
+ ccache --zero-stats
+ su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+ ccache --show-stats
upload_caches: ccache
test_world_script: |
@@ -325,7 +335,11 @@ task:
\
LLVM_CONFIG=${brewpath}/llvm/bin/llvm-config \
PYTHON=python3
- build_script: gmake -s -j${BUILD_JOBS} world-bin
+
+ build_script: |
+ ccache --zero-stats
+ gmake -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
upload_caches: ccache
test_world_script: |
@@ -421,7 +435,9 @@ task:
- perl src/tools/msvc/mkvcbuild.pl
build_script:
- vcvarsall x64
+ - ccache --zero-stats
- msbuild %MSBFLAGS% pgsql.sln
+ - ccache --show-stats
always:
upload_caches: ccache
@@ -536,7 +552,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# gcc, cassert on, dtrace off
always:
@@ -547,7 +565,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# clang, cassert off, dtrace off
always:
@@ -557,7 +577,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# clang, cassert on, dtrace on
always:
@@ -569,7 +591,9 @@ task:
${LINUX_CONFIGURE_FEATURES} \
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
# cross-compile to windows
always:
@@ -580,7 +604,9 @@ task:
CC="ccache x86_64-w64-mingw32-gcc" \
CXX="ccache x86_64-w64-mingw32-g++"
make -s -j${BUILD_JOBS} clean
+ ccache --zero-stats
time make -s -j${BUILD_JOBS} world-bin
+ ccache --show-stats
###
# Verify docs can be built
--
2.17.1
--STPqjqpCrtky8aYs
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0014-another-way-to-install-uri-regress-uri_regress.patch"
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Index AM API cleanup
@ 2024-11-27 12:57 Peter Eisentraut <[email protected]>
0 siblings, 2 replies; 14+ messages in thread
From: Peter Eisentraut @ 2024-11-27 12:57 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; Mark Dilger <[email protected]>; Kirill Reshke <[email protected]>; +Cc: pgsql-hackers; Alex Wang <[email protected]>
On 26.11.24 15:27, Andrew Dunstan wrote:
> On 2024-11-19 Tu 6:26 PM, Mark Dilger wrote:
>>> On Nov 16, 2024, at 9:10 AM, Kirill Reshke <[email protected]>
>>> wrote:
>>>
>>> Hi! Can we please have a rebased version of this patch series?
>> Sorry for the delay, and thanks for your interest. I will try to get
>> around to rebasing in the next few days.
>
> beat you to it :-)
Thanks for that.
I'm content to move forward with the approach of mapping to
RowCompareType, as we had already discussed.
I think, however, that we should rename RowCompareType. Otherwise, it's
just going to be confusing forevermore. I suggest to rename it simply
to CompareType.
What I really don't like is that there is ROWCOMPARE_INVALID *and*
ROWCOMPARE_NONE. No one is going to understand that and keep it
straight. I also don't really see a reason for having both. I tried
removing one of them, and the only thing that failed was the new code in
AlterOpFamilyAdd() that tries to use strategy_get_rctype() to detect
valid operator numbers. But that's new code that is not essential to
the purpose of this patch series. So I strongly suggest that we just
have ROWCOMPARE_INVALID (to mirror InvalidStrategy) with value 0. If
there is a reason to have a _NONE that is separate from that, we need to
think of a different interface.
We should make sure that gist is properly integrated into this
framework, give that we already have strategy number mapping logic
there. The current patch series does not provide mapping functions for
gist. We should add those. They should make use of
GistTranslateStratnum() (and we might need to add an inverse function).
The problem is that gist and GistTranslateStratnum() also need the
opclass, because the strategy numbers are not fixed for gist. So the
amtranslatestrategy and amtranslaterctype callbacks probably need to
expanded to cover that.
It might be that spgist has a similar requirement, I'm not sure. The
code in spgutils.c raises some doubts about what it's doing. I'm
curious why this patch set provides an implementation for spgist but not
gist. Is there anything interesting about spgist for this purpose?
I'm going to try to code up the gist support on top of this patch set to
make sure that it will fit well. I'll report back.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Index AM API cleanup
@ 2024-11-27 15:51 Mark Dilger <[email protected]>
parent: Peter Eisentraut <[email protected]>
1 sibling, 0 replies; 14+ messages in thread
From: Mark Dilger @ 2024-11-27 15:51 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Kirill Reshke <[email protected]>; pgsql-hackers; Alex Wang <[email protected]>
> On Nov 27, 2024, at 4:57 AM, Peter Eisentraut <[email protected]> wrote:
>
> On 26.11.24 15:27, Andrew Dunstan wrote:
>> On 2024-11-19 Tu 6:26 PM, Mark Dilger wrote:
>>>> On Nov 16, 2024, at 9:10 AM, Kirill Reshke <[email protected]> wrote:
>>>>
>>>> Hi! Can we please have a rebased version of this patch series?
>>> Sorry for the delay, and thanks for your interest. I will try to get around to rebasing in the next few days.
>> beat you to it :-)
>
> Thanks for that.
>
> I'm content to move forward with the approach of mapping to RowCompareType, as we had already discussed.
>
> I think, however, that we should rename RowCompareType. Otherwise, it's just going to be confusing forevermore. I suggest to rename it simply to CompareType.
I kept the name RowCompareType to avoid code churn, but go ahead and rename it if you prefer.
> What I really don't like is that there is ROWCOMPARE_INVALID *and* ROWCOMPARE_NONE. No one is going to understand that and keep it straight. I also don't really see a reason for having both. I tried removing one of them, and the only thing that failed was the new code in AlterOpFamilyAdd() that tries to use strategy_get_rctype() to detect valid operator numbers. But that's new code that is not essential to the purpose of this patch series. So I strongly suggest that we just have ROWCOMPARE_INVALID (to mirror InvalidStrategy) with value 0. If there is a reason to have a _NONE that is separate from that, we need to think of a different interface.
Yeah, those two can be combined if you like. I found the distinction between them useful during patch development, but I agree people will have a hard time understanding the difference. For the record, the difference is between "this index doesn't provide the requested functionality" vs. "you passed in an invalid parameter".
> We should make sure that gist is properly integrated into this framework, give that we already have strategy number mapping logic there. The current patch series does not provide mapping functions for gist. We should add those. They should make use of GistTranslateStratnum() (and we might need to add an inverse function). The problem is that gist and GistTranslateStratnum() also need the opclass, because the strategy numbers are not fixed for gist. So the amtranslatestrategy and amtranslaterctype callbacks probably need to expanded to cover that.
>
> It might be that spgist has a similar requirement, I'm not sure. The code in spgutils.c raises some doubts about what it's doing. I'm curious why this patch set provides an implementation for spgist but not gist. Is there anything interesting about spgist for this purpose?
That was experimental, as the code comment indicates:
+ /*
+ * Assume these have the semantics of =, !=, <, <=, >, >= as their
+ * names imply.
+ *
+ * TODO: Audit the behavior of these RTree strategies as used by spgist
+ * to determine if this assumption is appropriate.
+ */
In hindsight, this patch might have been less confusing if I had simply not provided the callbacks for spgist.
> I'm going to try to code up the gist support on top of this patch set to make sure that it will fit well. I'll report back.
The design of the patch allows indexes to simply not participate in the mapping. There's no requirement that an index provide these callbacks. So one approach is to just not touch gist and spgist and leave that for another day. However, if you want to verify that the interface is sufficient to meet gist's needs, then go ahead and try something out.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Index AM API cleanup
@ 2024-12-04 14:49 Peter Eisentraut <[email protected]>
parent: Peter Eisentraut <[email protected]>
1 sibling, 2 replies; 14+ messages in thread
From: Peter Eisentraut @ 2024-12-04 14:49 UTC (permalink / raw)
To: Mark Dilger <[email protected]>; +Cc: pgsql-hackers; Kirill Reshke <[email protected]>; Andrew Dunstan <[email protected]>; Alex Wang <[email protected]>; Paul A Jungwirth <[email protected]>
On 27.11.24 13:57, Peter Eisentraut wrote:
> I think, however, that we should rename RowCompareType. Otherwise, it's
> just going to be confusing forevermore. I suggest to rename it simply
> to CompareType.
> I'm going to try to code up the gist support on top of this patch set to
> make sure that it will fit well. I'll report back.
Here is a patch set in that direction. It renames RowCompareType to
CompareType and updates the surrounding commentary a bit. And then I'm
changing the gist strategy mapping to use the CompareType values instead
of the RT* strategy numbers. Seeing this now, I like this a lot better
than what we have now, because it makes it clearer in the API and the
code what is a real strategy number and what's a different kind of
thing. (This isn't entirely the above-mentioned integration of the gist
support into your patch set yet, but it's a meaningful part of it.)
From 9da99d3958cddb86ad98ea88687a13f08a12cf16 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 4 Dec 2024 14:03:24 +0100
Subject: [PATCH v19.1 1/2] Rename RowCompareType to CompareType
RowCompareType served as a way to describe the fundamental meaning of
an operator, notionally independent of an operator class (although so
far this was only really supported for btrees). Its original purpose
was for use inside RowCompareExpr, and it has also found some small
use outside, such as for get_op_btree_interpretation().
We want to expand this now, as a more general way to describe operator
semantics for other index access methods, including gist (to improve
GistTranslateStratnum()) and others not written yet. To avoid future
confusion, we rename the type to CompareType and the symbols from
ROWCOMPARE_XXX to COMPARE_XXX to reflect their more general purpose.
---
src/backend/executor/execExpr.c | 2 +-
src/backend/executor/execExprInterp.c | 12 ++++----
src/backend/executor/nodeIndexscan.c | 4 +--
src/backend/jit/llvm/llvmjit_expr.c | 12 ++++----
src/backend/optimizer/path/indxpath.c | 2 +-
src/backend/optimizer/util/predtest.c | 2 +-
src/backend/parser/parse_expr.c | 12 ++++----
src/backend/utils/cache/lsyscache.c | 6 ++--
src/include/executor/execExpr.h | 2 +-
src/include/nodes/primnodes.h | 41 ++++++++++++++++++---------
src/tools/pgindent/typedefs.list | 2 +-
11 files changed, 55 insertions(+), 42 deletions(-)
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index e0eb96fd5ad..afa0f6b14a6 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -2102,7 +2102,7 @@ ExecInitExprRec(Expr *node, ExprState *state,
/* Finally, examine the last comparison result */
scratch.opcode = EEOP_ROWCOMPARE_FINAL;
- scratch.d.rowcompare_final.rctype = rcexpr->rctype;
+ scratch.d.rowcompare_final.cmptype = rcexpr->cmptype;
ExprEvalPushStep(state, &scratch);
/* adjust jump targets */
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index bad7b195bfb..7ffb611ab8c 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -1438,22 +1438,22 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
EEO_CASE(EEOP_ROWCOMPARE_FINAL)
{
int32 cmpresult = DatumGetInt32(*op->resvalue);
- RowCompareType rctype = op->d.rowcompare_final.rctype;
+ CompareType cmptype = op->d.rowcompare_final.cmptype;
*op->resnull = false;
- switch (rctype)
+ switch (cmptype)
{
/* EQ and NE cases aren't allowed here */
- case ROWCOMPARE_LT:
+ case COMPARE_LT:
*op->resvalue = BoolGetDatum(cmpresult < 0);
break;
- case ROWCOMPARE_LE:
+ case COMPARE_LE:
*op->resvalue = BoolGetDatum(cmpresult <= 0);
break;
- case ROWCOMPARE_GE:
+ case COMPARE_GE:
*op->resvalue = BoolGetDatum(cmpresult >= 0);
break;
- case ROWCOMPARE_GT:
+ case COMPARE_GT:
*op->resvalue = BoolGetDatum(cmpresult > 0);
break;
default:
diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index 8000feff4c9..5377ce4ec30 100644
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -1344,7 +1344,7 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index,
&op_lefttype,
&op_righttype);
- if (op_strategy != rc->rctype)
+ if (op_strategy != rc->cmptype)
elog(ERROR, "RowCompare index qualification contains wrong operator");
opfuncid = get_opfamily_proc(opfamily,
@@ -1421,7 +1421,7 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index,
MemSet(this_scan_key, 0, sizeof(ScanKeyData));
this_scan_key->sk_flags = SK_ROW_HEADER;
this_scan_key->sk_attno = first_sub_key->sk_attno;
- this_scan_key->sk_strategy = rc->rctype;
+ this_scan_key->sk_strategy = rc->cmptype;
/* sk_subtype, sk_collation, sk_func not used in a header */
this_scan_key->sk_argument = PointerGetDatum(first_sub_key);
}
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index c533f552540..88bef871413 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -1770,7 +1770,7 @@ llvm_compile_expr(ExprState *state)
case EEOP_ROWCOMPARE_FINAL:
{
- RowCompareType rctype = op->d.rowcompare_final.rctype;
+ CompareType cmptype = op->d.rowcompare_final.cmptype;
LLVMValueRef v_cmpresult;
LLVMValueRef v_result;
@@ -1786,18 +1786,18 @@ llvm_compile_expr(ExprState *state)
l_load(b, TypeSizeT, v_resvaluep, ""),
LLVMInt32TypeInContext(lc), "");
- switch (rctype)
+ switch (cmptype)
{
- case ROWCOMPARE_LT:
+ case COMPARE_LT:
predicate = LLVMIntSLT;
break;
- case ROWCOMPARE_LE:
+ case COMPARE_LE:
predicate = LLVMIntSLE;
break;
- case ROWCOMPARE_GT:
+ case COMPARE_GT:
predicate = LLVMIntSGT;
break;
- case ROWCOMPARE_GE:
+ case COMPARE_GE:
predicate = LLVMIntSGE;
break;
default:
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 5d102a0d371..10f4c6a1f19 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -3675,7 +3675,7 @@ expand_indexqual_rowcompare(PlannerInfo *root,
{
RowCompareExpr *rc = makeNode(RowCompareExpr);
- rc->rctype = (RowCompareType) op_strategy;
+ rc->cmptype = (CompareType) op_strategy;
rc->opnos = new_ops;
rc->opfamilies = list_copy_head(clause->opfamilies,
matching_cols);
diff --git a/src/backend/optimizer/util/predtest.c b/src/backend/optimizer/util/predtest.c
index 0a132610140..e967d831bf1 100644
--- a/src/backend/optimizer/util/predtest.c
+++ b/src/backend/optimizer/util/predtest.c
@@ -1664,7 +1664,7 @@ clause_is_strict_for(Node *clause, Node *subexpr, bool allow_false)
#define BTEQ BTEqualStrategyNumber
#define BTGE BTGreaterEqualStrategyNumber
#define BTGT BTGreaterStrategyNumber
-#define BTNE ROWCOMPARE_NE
+#define BTNE COMPARE_NE
/* We use "none" for 0/false to make the tables align nicely */
#define none 0
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index c2806297aa4..f31e9a4e879 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -2807,7 +2807,7 @@ make_row_comparison_op(ParseState *pstate, List *opname,
List *largs, List *rargs, int location)
{
RowCompareExpr *rcexpr;
- RowCompareType rctype;
+ CompareType cmptype;
List *opexprs;
List *opnos;
List *opfamilies;
@@ -2928,15 +2928,15 @@ make_row_comparison_op(ParseState *pstate, List *opname,
errhint("Row comparison operators must be associated with btree operator families."),
parser_errposition(pstate, location)));
}
- rctype = (RowCompareType) i;
+ cmptype = (CompareType) i;
/*
* For = and <> cases, we just combine the pairwise operators with AND or
* OR respectively.
*/
- if (rctype == ROWCOMPARE_EQ)
+ if (cmptype == COMPARE_EQ)
return (Node *) makeBoolExpr(AND_EXPR, opexprs, location);
- if (rctype == ROWCOMPARE_NE)
+ if (cmptype == COMPARE_NE)
return (Node *) makeBoolExpr(OR_EXPR, opexprs, location);
/*
@@ -2953,7 +2953,7 @@ make_row_comparison_op(ParseState *pstate, List *opname,
{
OpBtreeInterpretation *opinfo = lfirst(j);
- if (opinfo->strategy == rctype)
+ if (opinfo->strategy == cmptype)
{
opfamily = opinfo->opfamily_id;
break;
@@ -2989,7 +2989,7 @@ make_row_comparison_op(ParseState *pstate, List *opname,
}
rcexpr = makeNode(RowCompareExpr);
- rcexpr->rctype = rctype;
+ rcexpr->cmptype = cmptype;
rcexpr->opnos = opnos;
rcexpr->opfamilies = opfamilies;
rcexpr->inputcollids = NIL; /* assign_expr_collations will fix this */
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index a85dc0d891f..af68dce0907 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -595,7 +595,7 @@ get_op_hash_functions(Oid opno,
*
* In addition to the normal btree operators, we consider a <> operator to be
* a "member" of an opfamily if its negator is an equality operator of the
- * opfamily. ROWCOMPARE_NE is returned as the strategy number for this case.
+ * opfamily. COMPARE_NE is returned as the strategy number for this case.
*/
List *
get_op_btree_interpretation(Oid opno)
@@ -666,11 +666,11 @@ get_op_btree_interpretation(Oid opno)
if (op_strategy != BTEqualStrategyNumber)
continue;
- /* OK, report it with "strategy" ROWCOMPARE_NE */
+ /* OK, report it with "strategy" COMPARE_NE */
thisresult = (OpBtreeInterpretation *)
palloc(sizeof(OpBtreeInterpretation));
thisresult->opfamily_id = op_form->amopfamily;
- thisresult->strategy = ROWCOMPARE_NE;
+ thisresult->strategy = COMPARE_NE;
thisresult->oplefttype = op_form->amoplefttype;
thisresult->oprighttype = op_form->amoprighttype;
result = lappend(result, thisresult);
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index 56fb0d0adbe..42db1161da1 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -494,7 +494,7 @@ typedef struct ExprEvalStep
/* for EEOP_ROWCOMPARE_FINAL */
struct
{
- RowCompareType rctype;
+ CompareType cmptype;
} rowcompare_final;
/* for EEOP_MINMAX */
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index b0ef1952e8f..107eb36a340 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1435,6 +1435,31 @@ typedef struct RowExpr
ParseLoc location; /* token location, or -1 if unknown */
} RowExpr;
+/*
+ * CompareType - fundamental semantics of certain operators
+ *
+ * These enum symbols represent the fundamental semantics of certain operators
+ * that the system needs to have some hardcoded knowledge about. (For
+ * example, RowCompareExpr needs to know which operators can be determined to
+ * act like =, <>, <, etc.) Index access methods map (some of) strategy
+ * numbers to these values so that the system can know about the meaning of
+ * (some of) the operators without needing hardcoded knowledge of index AM's
+ * strategy numbering.
+ *
+ * XXX Currently, this mapping is not fully developed and the values are
+ * chosen to match btree strategy numbers, which is not going to work very
+ * well for other access methods.
+ */
+typedef enum CompareType
+{
+ COMPARE_LT = 1, /* BTLessStrategyNumber */
+ COMPARE_LE = 2, /* BTLessEqualStrategyNumber */
+ COMPARE_EQ = 3, /* BTEqualStrategyNumber */
+ COMPARE_GE = 4, /* BTGreaterEqualStrategyNumber */
+ COMPARE_GT = 5, /* BTGreaterStrategyNumber */
+ COMPARE_NE = 6, /* no such btree strategy */
+} CompareType;
+
/*
* RowCompareExpr - row-wise comparison, such as (a, b) <= (1, 2)
*
@@ -1446,26 +1471,14 @@ typedef struct RowExpr
*
* A RowCompareExpr node is only generated for the < <= > >= cases;
* the = and <> cases are translated to simple AND or OR combinations
- * of the pairwise comparisons. However, we include = and <> in the
- * RowCompareType enum for the convenience of parser logic.
+ * of the pairwise comparisons.
*/
-typedef enum RowCompareType
-{
- /* Values of this enum are chosen to match btree strategy numbers */
- ROWCOMPARE_LT = 1, /* BTLessStrategyNumber */
- ROWCOMPARE_LE = 2, /* BTLessEqualStrategyNumber */
- ROWCOMPARE_EQ = 3, /* BTEqualStrategyNumber */
- ROWCOMPARE_GE = 4, /* BTGreaterEqualStrategyNumber */
- ROWCOMPARE_GT = 5, /* BTGreaterStrategyNumber */
- ROWCOMPARE_NE = 6, /* no such btree strategy */
-} RowCompareType;
-
typedef struct RowCompareExpr
{
Expr xpr;
/* LT LE GE or GT, never EQ or NE */
- RowCompareType rctype;
+ CompareType cmptype;
/* OID list of pairwise comparison ops */
List *opnos pg_node_attr(query_jumble_ignore);
/* OID list of containing operator families */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 2d4c870423a..4100e634964 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -455,6 +455,7 @@ CommitTimestampShared
CommonEntry
CommonTableExpr
CompareScalarsContext
+CompareType
CompiledExprState
CompositeIOData
CompositeTypeStmt
@@ -2487,7 +2488,6 @@ RoleSpecType
RoleStmtType
RollupData
RowCompareExpr
-RowCompareType
RowExpr
RowIdentityVarInfo
RowMarkClause
base-commit: 7727049e8f663344d4d0457e1d9ec048d626f3d9
--
2.47.1
From 9509de62880094562eb234e4bdecbaed5062ff91 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 4 Dec 2024 15:31:18 +0100
Subject: [PATCH v19.1 2/2] Change gist stratnum function to use CompareType
This changes commit 7406ab623fe in that the gist strategy number
mapping support function is changed to use the CompareType enum as
input, instead of the "well-known" RT*StrategyNumber strategy numbers.
This is a bit cleaner, since you are not dealing with two sets of
strategy numbers. Also, this will enable us to subsume this system
into a more general system of using CompareType to define operator
semantics across index methods.
---
contrib/btree_gist/btree_gist--1.7--1.8.sql | 54 ++++++++++----------
contrib/btree_gist/btree_gist.c | 15 +++---
contrib/btree_gist/expected/stratnum.out | 4 +-
contrib/btree_gist/sql/stratnum.sql | 4 +-
doc/src/sgml/gist.sgml | 24 ++++-----
doc/src/sgml/xindex.sgml | 2 +-
src/backend/access/gist/gistutil.c | 35 +++++++++----
src/backend/access/gist/gistvalidate.c | 2 +-
src/backend/catalog/pg_constraint.c | 20 ++++----
src/backend/commands/indexcmds.c | 50 +++++++++---------
src/backend/commands/tablecmds.c | 10 ++--
src/include/access/gist.h | 3 +-
src/include/catalog/pg_amproc.dat | 12 ++---
src/include/catalog/pg_proc.dat | 6 +--
src/include/commands/defrem.h | 4 +-
src/include/nodes/primnodes.h | 4 +-
src/test/regress/expected/misc_functions.out | 16 +++---
src/test/regress/sql/misc_functions.sql | 4 +-
18 files changed, 144 insertions(+), 125 deletions(-)
diff --git a/contrib/btree_gist/btree_gist--1.7--1.8.sql b/contrib/btree_gist/btree_gist--1.7--1.8.sql
index 307bfe574b0..c702426deab 100644
--- a/contrib/btree_gist/btree_gist--1.7--1.8.sql
+++ b/contrib/btree_gist/btree_gist--1.7--1.8.sql
@@ -3,85 +3,85 @@
-- complain if script is sourced in psql, rather than via CREATE EXTENSION
\echo Use "ALTER EXTENSION btree_gist UPDATE TO '1.8'" to load this file. \quit
-CREATE FUNCTION gist_stratnum_btree(smallint)
+CREATE FUNCTION gist_stratnum_btree(int)
RETURNS smallint
AS 'MODULE_PATHNAME'
LANGUAGE C IMMUTABLE PARALLEL SAFE STRICT;
ALTER OPERATOR FAMILY gist_oid_ops USING gist ADD
- FUNCTION 12 (oid, oid) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (oid, oid) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_int2_ops USING gist ADD
- FUNCTION 12 (int2, int2) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (int2, int2) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_int4_ops USING gist ADD
- FUNCTION 12 (int4, int4) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (int4, int4) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_int8_ops USING gist ADD
- FUNCTION 12 (int8, int8) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (int8, int8) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_float4_ops USING gist ADD
- FUNCTION 12 (float4, float4) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (float4, float4) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_float8_ops USING gist ADD
- FUNCTION 12 (float8, float8) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (float8, float8) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_timestamp_ops USING gist ADD
- FUNCTION 12 (timestamp, timestamp) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (timestamp, timestamp) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_timestamptz_ops USING gist ADD
- FUNCTION 12 (timestamptz, timestamptz) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (timestamptz, timestamptz) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_time_ops USING gist ADD
- FUNCTION 12 (time, time) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (time, time) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_date_ops USING gist ADD
- FUNCTION 12 (date, date) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (date, date) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_interval_ops USING gist ADD
- FUNCTION 12 (interval, interval) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (interval, interval) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_cash_ops USING gist ADD
- FUNCTION 12 (money, money) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (money, money) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_macaddr_ops USING gist ADD
- FUNCTION 12 (macaddr, macaddr) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (macaddr, macaddr) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_text_ops USING gist ADD
- FUNCTION 12 (text, text) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (text, text) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_bpchar_ops USING gist ADD
- FUNCTION 12 (bpchar, bpchar) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (bpchar, bpchar) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_bytea_ops USING gist ADD
- FUNCTION 12 (bytea, bytea) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (bytea, bytea) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_numeric_ops USING gist ADD
- FUNCTION 12 (numeric, numeric) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (numeric, numeric) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_bit_ops USING gist ADD
- FUNCTION 12 (bit, bit) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (bit, bit) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_vbit_ops USING gist ADD
- FUNCTION 12 (varbit, varbit) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (varbit, varbit) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_inet_ops USING gist ADD
- FUNCTION 12 (inet, inet) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (inet, inet) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_cidr_ops USING gist ADD
- FUNCTION 12 (cidr, cidr) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (cidr, cidr) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_timetz_ops USING gist ADD
- FUNCTION 12 (timetz, timetz) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (timetz, timetz) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_uuid_ops USING gist ADD
- FUNCTION 12 (uuid, uuid) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (uuid, uuid) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_macaddr8_ops USING gist ADD
- FUNCTION 12 (macaddr8, macaddr8) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (macaddr8, macaddr8) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_enum_ops USING gist ADD
- FUNCTION 12 (anyenum, anyenum) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (anyenum, anyenum) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_bool_ops USING gist ADD
- FUNCTION 12 (bool, bool) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (bool, bool) gist_stratnum_btree (int) ;
diff --git a/contrib/btree_gist/btree_gist.c b/contrib/btree_gist/btree_gist.c
index 5fd4cce27d0..fc6c7091795 100644
--- a/contrib/btree_gist/btree_gist.c
+++ b/contrib/btree_gist/btree_gist.c
@@ -4,6 +4,7 @@
#include "postgres.h"
#include "access/stratnum.h"
+#include "nodes/primnodes.h"
#include "utils/builtins.h"
PG_MODULE_MAGIC;
@@ -60,19 +61,19 @@ gbt_decompress(PG_FUNCTION_ARGS)
Datum
gist_stratnum_btree(PG_FUNCTION_ARGS)
{
- StrategyNumber strat = PG_GETARG_UINT16(0);
+ CompareType cmptype = PG_GETARG_INT32(0);
- switch (strat)
+ switch (cmptype)
{
- case RTEqualStrategyNumber:
+ case COMPARE_EQ:
PG_RETURN_UINT16(BTEqualStrategyNumber);
- case RTLessStrategyNumber:
+ case COMPARE_LT:
PG_RETURN_UINT16(BTLessStrategyNumber);
- case RTLessEqualStrategyNumber:
+ case COMPARE_LE:
PG_RETURN_UINT16(BTLessEqualStrategyNumber);
- case RTGreaterStrategyNumber:
+ case COMPARE_GT:
PG_RETURN_UINT16(BTGreaterStrategyNumber);
- case RTGreaterEqualStrategyNumber:
+ case COMPARE_GE:
PG_RETURN_UINT16(BTGreaterEqualStrategyNumber);
default:
PG_RETURN_UINT16(InvalidStrategy);
diff --git a/contrib/btree_gist/expected/stratnum.out b/contrib/btree_gist/expected/stratnum.out
index 9d80c6590d9..dd0edaf4a20 100644
--- a/contrib/btree_gist/expected/stratnum.out
+++ b/contrib/btree_gist/expected/stratnum.out
@@ -1,11 +1,11 @@
-- test stratnum support func
-SELECT gist_stratnum_btree(3::smallint);
+SELECT gist_stratnum_btree(7);
gist_stratnum_btree
---------------------
0
(1 row)
-SELECT gist_stratnum_btree(18::smallint);
+SELECT gist_stratnum_btree(3);
gist_stratnum_btree
---------------------
3
diff --git a/contrib/btree_gist/sql/stratnum.sql b/contrib/btree_gist/sql/stratnum.sql
index f58cdbe93da..75adddad849 100644
--- a/contrib/btree_gist/sql/stratnum.sql
+++ b/contrib/btree_gist/sql/stratnum.sql
@@ -1,3 +1,3 @@
-- test stratnum support func
-SELECT gist_stratnum_btree(3::smallint);
-SELECT gist_stratnum_btree(18::smallint);
+SELECT gist_stratnum_btree(7);
+SELECT gist_stratnum_btree(3);
diff --git a/doc/src/sgml/gist.sgml b/doc/src/sgml/gist.sgml
index 638d912dc2d..99098ab2522 100644
--- a/doc/src/sgml/gist.sgml
+++ b/doc/src/sgml/gist.sgml
@@ -290,8 +290,8 @@ <title>Extensibility</title>
The optional eleventh method <function>sortsupport</function> is used to
speed up building a <acronym>GiST</acronym> index.
The optional twelfth method <function>stratnum</function> is used to
- translate well-known <literal>RT*StrategyNumber</literal>s (from
- <filename>src/include/access/stratnum.h</filename>) into strategy numbers
+ translate compare types (from
+ <filename>src/include/nodes/primnodes.h</filename>) into strategy numbers
used by the operator class. This lets the core code look up operators for
temporal constraint indexes.
</para>
@@ -1173,8 +1173,8 @@ <title>Extensibility</title>
<term><function>stratnum</function></term>
<listitem>
<para>
- Given an <literal>RT*StrategyNumber</literal> value from
- <filename>src/include/access/stratnum.h</filename>, returns a strategy
+ Given a <literal>CompareType</literal> value from
+ <filename>src/include/nodes/primnodes.h</filename>, returns a strategy
number used by this operator class for matching functionality. The
function should return <literal>InvalidStrategy</literal> if the
operator class has no matching strategy.
@@ -1184,7 +1184,7 @@ <title>Extensibility</title>
This is used for temporal index constraints (i.e., <literal>PRIMARY
KEY</literal> and <literal>UNIQUE</literal>). If the operator class
provides this function and it returns results for
- <literal>RTEqualStrategyNumber</literal>, it can be used in the
+ <literal>COMPARE_EQ</literal>, it can be used in the
non-<literal>WITHOUT OVERLAPS</literal> part(s) of an index constraint.
</para>
@@ -1194,7 +1194,7 @@ <title>Extensibility</title>
<programlisting>
CREATE OR REPLACE FUNCTION my_stratnum(integer)
-RETURNS integer
+RETURNS smallint
AS 'MODULE_PATHNAME'
LANGUAGE C STRICT;
</programlisting>
@@ -1209,12 +1209,12 @@ <title>Extensibility</title>
Datum
my_stratnum(PG_FUNCTION_ARGS)
{
- StrategyNumber strategy = (StrategyNumber) PG_GETARG_UINT16(1);
+ CompareType cmptype = PG_GETARG_INT32(0);
StrategyNumber ret = InvalidStrategy;
- switch (strategy)
+ switch (cmptype)
{
- case RTEqualStrategyNumber:
+ case COMPARE_EQ:
ret = BTEqualStrategyNumber;
}
@@ -1226,9 +1226,9 @@ <title>Extensibility</title>
<para>
One translation function is provided by
<productname>PostgreSQL</productname>:
- <literal>gist_stratnum_identity</literal> is for operator classes that
- already use the <literal>RT*StrategyNumber</literal> constants. It
- returns whatever is passed to it. The <literal>btree_gist</literal>
+ <literal>gist_stratnum_common</literal> is for operator classes that
+ use the <literal>RT*StrategyNumber</literal> constants.
+ The <literal>btree_gist</literal>
extension defines a second translation function,
<literal>gist_stratnum_btree</literal>, for operator classes that use
the <literal>BT*StrategyNumber</literal> constants.
diff --git a/doc/src/sgml/xindex.sgml b/doc/src/sgml/xindex.sgml
index 3a19dab15e0..05361962495 100644
--- a/doc/src/sgml/xindex.sgml
+++ b/doc/src/sgml/xindex.sgml
@@ -592,7 +592,7 @@ <title>GiST Support Functions</title>
</row>
<row>
<entry><function>stratnum</function></entry>
- <entry>translate well-known strategy numbers to ones
+ <entry>translate compare types to strategy numbers
used by the operator class (optional)</entry>
<entry>12</entry>
</row>
diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c
index d2d0b36d4ea..bde541a6251 100644
--- a/src/backend/access/gist/gistutil.c
+++ b/src/backend/access/gist/gistutil.c
@@ -1058,27 +1058,44 @@ gistGetFakeLSN(Relation rel)
}
/*
- * Returns the same number that was received.
- *
- * This is for GiST opclasses that use the RT*StrategyNumber constants.
+ * This is a stratnum support function for GiST opclasses that use the
+ * RT*StrategyNumber constants.
*/
Datum
-gist_stratnum_identity(PG_FUNCTION_ARGS)
+gist_stratnum_common(PG_FUNCTION_ARGS)
{
- StrategyNumber strat = PG_GETARG_UINT16(0);
+ CompareType cmptype = PG_GETARG_INT32(0);
- PG_RETURN_UINT16(strat);
+ switch (cmptype)
+ {
+ case COMPARE_EQ:
+ PG_RETURN_UINT16(RTEqualStrategyNumber);
+ case COMPARE_LT:
+ PG_RETURN_UINT16(RTLessStrategyNumber);
+ case COMPARE_LE:
+ PG_RETURN_UINT16(RTLessEqualStrategyNumber);
+ case COMPARE_GT:
+ PG_RETURN_UINT16(RTGreaterStrategyNumber);
+ case COMPARE_GE:
+ PG_RETURN_UINT16(RTGreaterEqualStrategyNumber);
+ case COMPARE_OVERLAP:
+ PG_RETURN_UINT16(RTOverlapStrategyNumber);
+ case COMPARE_CONTAINED_BY:
+ PG_RETURN_UINT16(RTContainedByStrategyNumber);
+ default:
+ PG_RETURN_UINT16(InvalidStrategy);
+ }
}
/*
- * Returns the opclass's private stratnum used for the given strategy.
+ * Returns the opclass's private stratnum used for the given compare type.
*
* Calls the opclass's GIST_STRATNUM_PROC support function, if any,
* and returns the result.
* Returns InvalidStrategy if the function is not defined.
*/
StrategyNumber
-GistTranslateStratnum(Oid opclass, StrategyNumber strat)
+GistTranslateStratnum(Oid opclass, CompareType cmptype)
{
Oid opfamily;
Oid opcintype;
@@ -1095,6 +1112,6 @@ GistTranslateStratnum(Oid opclass, StrategyNumber strat)
return InvalidStrategy;
/* Ask the translation function */
- result = OidFunctionCall1Coll(funcid, InvalidOid, UInt16GetDatum(strat));
+ result = OidFunctionCall1Coll(funcid, InvalidOid, Int32GetDatum(cmptype));
return DatumGetUInt16(result);
}
diff --git a/src/backend/access/gist/gistvalidate.c b/src/backend/access/gist/gistvalidate.c
index 0901543a60a..7d51fc167d6 100644
--- a/src/backend/access/gist/gistvalidate.c
+++ b/src/backend/access/gist/gistvalidate.c
@@ -148,7 +148,7 @@ gistvalidate(Oid opclassoid)
break;
case GIST_STRATNUM_PROC:
ok = check_amproc_signature(procform->amproc, INT2OID, true,
- 1, 1, INT2OID);
+ 1, 1, INT4OID);
break;
default:
ereport(INFO,
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 9c05a98d28c..4a076e2874e 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -1638,22 +1638,22 @@ FindFKPeriodOpers(Oid opclass,
* of the old value, then we can treat the attribute as if it didn't
* change, and skip the RI check.
*/
- strat = RTContainedByStrategyNumber;
- GetOperatorFromWellKnownStrategy(opclass,
- InvalidOid,
- containedbyoperoid,
- &strat);
+ GetOperatorFromCompareType(opclass,
+ InvalidOid,
+ COMPARE_CONTAINED_BY,
+ containedbyoperoid,
+ &strat);
/*
* Now look up the ContainedBy operator. Its left arg must be the type of
* the column (or rather of the opclass). Its right arg must match the
* return type of the support proc.
*/
- strat = RTContainedByStrategyNumber;
- GetOperatorFromWellKnownStrategy(opclass,
- ANYMULTIRANGEOID,
- aggedcontainedbyoperoid,
- &strat);
+ GetOperatorFromCompareType(opclass,
+ ANYMULTIRANGEOID,
+ COMPARE_CONTAINED_BY,
+ aggedcontainedbyoperoid,
+ &strat);
}
/*
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 4049ce1a10f..9c1a159754c 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2178,15 +2178,15 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
}
else if (iswithoutoverlaps)
{
+ CompareType cmptype;
StrategyNumber strat;
Oid opid;
if (attn == nkeycols - 1)
- strat = RTOverlapStrategyNumber;
+ cmptype = COMPARE_OVERLAP;
else
- strat = RTEqualStrategyNumber;
- GetOperatorFromWellKnownStrategy(opclassOids[attn], InvalidOid,
- &opid, &strat);
+ cmptype = COMPARE_EQ;
+ GetOperatorFromCompareType(opclassOids[attn], InvalidOid, cmptype, &opid, &strat);
indexInfo->ii_ExclusionOps[attn] = opid;
indexInfo->ii_ExclusionProcs[attn] = get_opcode(opid);
indexInfo->ii_ExclusionStrats[attn] = strat;
@@ -2422,30 +2422,28 @@ GetDefaultOpClass(Oid type_id, Oid am_id)
}
/*
- * GetOperatorFromWellKnownStrategy
+ * GetOperatorFromCompareType
*
* opclass - the opclass to use
* rhstype - the type for the right-hand side, or InvalidOid to use the type of the given opclass.
+ * cmptype - kind of operator to find
* opid - holds the operator we found
- * strat - holds the input and output strategy number
+ * strat - holds the output strategy number
*
- * Finds an operator from a "well-known" strategy number. This is used for
- * temporal index constraints (and other temporal features) to look up
- * equality and overlaps operators, since the strategy numbers for non-btree
- * indexams need not follow any fixed scheme. We ask an opclass support
- * function to translate from the well-known number to the internal value. If
- * the function isn't defined or it gives no result, we return
- * InvalidStrategy.
+ * Finds an operator from a CompareType. This is used for temporal index
+ * constraints (and other temporal features) to look up equality and overlaps
+ * operators. We ask an opclass support function to translate from the
+ * compare type to the internal strategy numbers. If the function isn't
+ * defined or it gives no result, we set *strat to InvalidStrategy.
*/
void
-GetOperatorFromWellKnownStrategy(Oid opclass, Oid rhstype,
- Oid *opid, StrategyNumber *strat)
+GetOperatorFromCompareType(Oid opclass, Oid rhstype, CompareType cmptype,
+ Oid *opid, StrategyNumber *strat)
{
Oid opfamily;
Oid opcintype;
- StrategyNumber instrat = *strat;
- Assert(instrat == RTEqualStrategyNumber || instrat == RTOverlapStrategyNumber || instrat == RTContainedByStrategyNumber);
+ Assert(cmptype == COMPARE_EQ || cmptype == COMPARE_OVERLAP || cmptype == COMPARE_CONTAINED_BY);
*opid = InvalidOid;
@@ -2457,7 +2455,7 @@ GetOperatorFromWellKnownStrategy(Oid opclass, Oid rhstype,
* For now we only need GiST support, but this could support other
* indexams if we wanted.
*/
- *strat = GistTranslateStratnum(opclass, instrat);
+ *strat = GistTranslateStratnum(opclass, cmptype);
if (*strat == InvalidStrategy)
{
HeapTuple tuple;
@@ -2468,11 +2466,11 @@ GetOperatorFromWellKnownStrategy(Oid opclass, Oid rhstype,
ereport(ERROR,
errcode(ERRCODE_UNDEFINED_OBJECT),
- instrat == RTEqualStrategyNumber ? errmsg("could not identify an equality operator for type %s", format_type_be(opcintype)) :
- instrat == RTOverlapStrategyNumber ? errmsg("could not identify an overlaps operator for type %s", format_type_be(opcintype)) :
- instrat == RTContainedByStrategyNumber ? errmsg("could not identify a contained-by operator for type %s", format_type_be(opcintype)) : 0,
- errdetail("Could not translate strategy number %d for operator class \"%s\" for access method \"%s\".",
- instrat, NameStr(((Form_pg_opclass) GETSTRUCT(tuple))->opcname), "gist"));
+ cmptype = COMPARE_EQ ? errmsg("could not identify an equality operator for type %s", format_type_be(opcintype)) :
+ cmptype == COMPARE_OVERLAP ? errmsg("could not identify an overlaps operator for type %s", format_type_be(opcintype)) :
+ cmptype == COMPARE_CONTAINED_BY ? errmsg("could not identify a contained-by operator for type %s", format_type_be(opcintype)) : 0,
+ errdetail("Could not translate compare type %d for operator class \"%s\" for access method \"%s\".",
+ cmptype, NameStr(((Form_pg_opclass) GETSTRUCT(tuple))->opcname), "gist"));
}
/*
@@ -2495,9 +2493,9 @@ GetOperatorFromWellKnownStrategy(Oid opclass, Oid rhstype,
ereport(ERROR,
errcode(ERRCODE_UNDEFINED_OBJECT),
- instrat == RTEqualStrategyNumber ? errmsg("could not identify an equality operator for type %s", format_type_be(opcintype)) :
- instrat == RTOverlapStrategyNumber ? errmsg("could not identify an overlaps operator for type %s", format_type_be(opcintype)) :
- instrat == RTContainedByStrategyNumber ? errmsg("could not identify a contained-by operator for type %s", format_type_be(opcintype)) : 0,
+ cmptype == COMPARE_EQ ? errmsg("could not identify an equality operator for type %s", format_type_be(opcintype)) :
+ cmptype == COMPARE_OVERLAP ? errmsg("could not identify an overlaps operator for type %s", format_type_be(opcintype)) :
+ cmptype == COMPARE_CONTAINED_BY ? errmsg("could not identify a contained-by operator for type %s", format_type_be(opcintype)) : 0,
errdetail("There is no suitable operator in operator family \"%s\" for access method \"%s\".",
NameStr(((Form_pg_opfamily) GETSTRUCT(tuple))->opfname), "gist"));
}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 6ccae4cb4a8..aef672d278d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9972,7 +9972,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
if (with_period)
{
- StrategyNumber rtstrategy;
+ CompareType cmptype;
bool for_overlaps = with_period && i == numpks - 1;
/*
@@ -9982,14 +9982,14 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
if (amid != GIST_AM_OID)
elog(ERROR, "only GiST indexes are supported for temporal foreign keys");
- rtstrategy = for_overlaps ? RTOverlapStrategyNumber : RTEqualStrategyNumber;
+ cmptype = for_overlaps ? COMPARE_OVERLAP : COMPARE_EQ;
/*
* An opclass can use whatever strategy numbers it wants, so we
* ask the opclass what number it actually uses instead of our RT*
* constants.
*/
- eqstrategy = GistTranslateStratnum(opclasses[i], rtstrategy);
+ eqstrategy = GistTranslateStratnum(opclasses[i], cmptype);
if (eqstrategy == InvalidStrategy)
{
HeapTuple tuple;
@@ -10003,8 +10003,8 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
for_overlaps
? errmsg("could not identify an overlaps operator for foreign key")
: errmsg("could not identify an equality operator for foreign key"),
- errdetail("Could not translate strategy number %d for operator class \"%s\" for access method \"%s\".",
- rtstrategy, NameStr(((Form_pg_opclass) GETSTRUCT(tuple))->opcname), "gist"));
+ errdetail("Could not translate compare type %d for operator class \"%s\" for access method \"%s\".",
+ cmptype, NameStr(((Form_pg_opclass) GETSTRUCT(tuple))->opcname), "gist"));
}
}
else
diff --git a/src/include/access/gist.h b/src/include/access/gist.h
index 22dd04c1418..a8689cfcefe 100644
--- a/src/include/access/gist.h
+++ b/src/include/access/gist.h
@@ -247,6 +247,7 @@ typedef struct
do { (e).key = (k); (e).rel = (r); (e).page = (pg); \
(e).offset = (o); (e).leafkey = (l); } while (0)
-extern StrategyNumber GistTranslateStratnum(Oid opclass, StrategyNumber strat);
+enum CompareType;
+extern StrategyNumber GistTranslateStratnum(Oid opclass, enum CompareType cmp);
#endif /* GIST_H */
diff --git a/src/include/catalog/pg_amproc.dat b/src/include/catalog/pg_amproc.dat
index 5d7fe292bf6..a04828fde96 100644
--- a/src/include/catalog/pg_amproc.dat
+++ b/src/include/catalog/pg_amproc.dat
@@ -508,7 +508,7 @@
amprocrighttype => 'box', amprocnum => '8', amproc => 'gist_box_distance' },
{ amprocfamily => 'gist/box_ops', amproclefttype => 'box',
amprocrighttype => 'box', amprocnum => '12',
- amproc => 'gist_stratnum_identity' },
+ amproc => 'gist_stratnum_common' },
{ amprocfamily => 'gist/poly_ops', amproclefttype => 'polygon',
amprocrighttype => 'polygon', amprocnum => '1',
amproc => 'gist_poly_consistent' },
@@ -530,7 +530,7 @@
amproc => 'gist_poly_distance' },
{ amprocfamily => 'gist/poly_ops', amproclefttype => 'polygon',
amprocrighttype => 'polygon', amprocnum => '12',
- amproc => 'gist_stratnum_identity' },
+ amproc => 'gist_stratnum_common' },
{ amprocfamily => 'gist/circle_ops', amproclefttype => 'circle',
amprocrighttype => 'circle', amprocnum => '1',
amproc => 'gist_circle_consistent' },
@@ -551,7 +551,7 @@
amproc => 'gist_circle_distance' },
{ amprocfamily => 'gist/circle_ops', amproclefttype => 'circle',
amprocrighttype => 'circle', amprocnum => '12',
- amproc => 'gist_stratnum_identity' },
+ amproc => 'gist_stratnum_common' },
{ amprocfamily => 'gist/tsvector_ops', amproclefttype => 'tsvector',
amprocrighttype => 'tsvector', amprocnum => '1',
amproc => 'gtsvector_consistent(internal,tsvector,int2,oid,internal)' },
@@ -608,7 +608,7 @@
amproc => 'range_gist_same' },
{ amprocfamily => 'gist/range_ops', amproclefttype => 'anyrange',
amprocrighttype => 'anyrange', amprocnum => '12',
- amproc => 'gist_stratnum_identity' },
+ amproc => 'gist_stratnum_common' },
{ amprocfamily => 'gist/network_ops', amproclefttype => 'inet',
amprocrighttype => 'inet', amprocnum => '1',
amproc => 'inet_gist_consistent' },
@@ -627,7 +627,7 @@
amprocrighttype => 'inet', amprocnum => '9', amproc => 'inet_gist_fetch' },
{ amprocfamily => 'gist/network_ops', amproclefttype => 'inet',
amprocrighttype => 'inet', amprocnum => '12',
- amproc => 'gist_stratnum_identity' },
+ amproc => 'gist_stratnum_common' },
{ amprocfamily => 'gist/multirange_ops', amproclefttype => 'anymultirange',
amprocrighttype => 'anymultirange', amprocnum => '1',
amproc => 'multirange_gist_consistent' },
@@ -648,7 +648,7 @@
amproc => 'range_gist_same' },
{ amprocfamily => 'gist/multirange_ops', amproclefttype => 'anymultirange',
amprocrighttype => 'anymultirange', amprocnum => '12',
- amproc => 'gist_stratnum_identity' },
+ amproc => 'gist_stratnum_common' },
# gin
{ amprocfamily => 'gin/array_ops', amproclefttype => 'anyarray',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 9575524007f..988b8a336f7 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -12403,8 +12403,8 @@
# GiST stratnum implementations
{ oid => '8047', descr => 'GiST support',
- proname => 'gist_stratnum_identity', prorettype => 'int2',
- proargtypes => 'int2',
- prosrc => 'gist_stratnum_identity' },
+ proname => 'gist_stratnum_common', prorettype => 'int2',
+ proargtypes => 'int4',
+ prosrc => 'gist_stratnum_common' },
]
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 5fd095ea177..bf44d111c00 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -50,8 +50,8 @@ extern bool CheckIndexCompatible(Oid oldId,
extern Oid GetDefaultOpClass(Oid type_id, Oid am_id);
extern Oid ResolveOpClass(const List *opclass, Oid attrType,
const char *accessMethodName, Oid accessMethodId);
-extern void GetOperatorFromWellKnownStrategy(Oid opclass, Oid rhstype,
- Oid *opid, StrategyNumber *strat);
+extern void GetOperatorFromCompareType(Oid opclass, Oid rhstype, CompareType cmptype,
+ Oid *opid, StrategyNumber *strat);
/* commands/functioncmds.c */
extern ObjectAddress CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt);
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 107eb36a340..60a8c62fca4 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1446,7 +1446,7 @@ typedef struct RowExpr
* (some of) the operators without needing hardcoded knowledge of index AM's
* strategy numbering.
*
- * XXX Currently, this mapping is not fully developed and the values are
+ * XXX Currently, this mapping is not fully developed and most values are
* chosen to match btree strategy numbers, which is not going to work very
* well for other access methods.
*/
@@ -1458,6 +1458,8 @@ typedef enum CompareType
COMPARE_GE = 4, /* BTGreaterEqualStrategyNumber */
COMPARE_GT = 5, /* BTGreaterStrategyNumber */
COMPARE_NE = 6, /* no such btree strategy */
+ COMPARE_OVERLAP,
+ COMPARE_CONTAINED_BY,
} CompareType;
/*
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 8c73d2b621e..dc28fcb310e 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -891,15 +891,15 @@ SELECT pg_column_toast_chunk_id(a) IS NULL,
DROP TABLE test_chunk_id;
DROP FUNCTION explain_mask_costs(text, bool, bool, bool, bool);
-- test stratnum support functions
-SELECT gist_stratnum_identity(3::smallint);
- gist_stratnum_identity
-------------------------
- 3
+SELECT gist_stratnum_common(7);
+ gist_stratnum_common
+----------------------
+ 3
(1 row)
-SELECT gist_stratnum_identity(18::smallint);
- gist_stratnum_identity
-------------------------
- 18
+SELECT gist_stratnum_common(3);
+ gist_stratnum_common
+----------------------
+ 18
(1 row)
diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql
index 0a0c6e5a6bb..81ef45d128c 100644
--- a/src/test/regress/sql/misc_functions.sql
+++ b/src/test/regress/sql/misc_functions.sql
@@ -401,5 +401,5 @@ CREATE TABLE test_chunk_id (a TEXT, b TEXT STORAGE EXTERNAL);
DROP FUNCTION explain_mask_costs(text, bool, bool, bool, bool);
-- test stratnum support functions
-SELECT gist_stratnum_identity(3::smallint);
-SELECT gist_stratnum_identity(18::smallint);
+SELECT gist_stratnum_common(7);
+SELECT gist_stratnum_common(3);
--
2.47.1
Attachments:
[text/plain] v19.1-0001-Rename-RowCompareType-to-CompareType.patch (12.5K, ../../[email protected]/2-v19.1-0001-Rename-RowCompareType-to-CompareType.patch)
download | inline diff:
From 9da99d3958cddb86ad98ea88687a13f08a12cf16 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 4 Dec 2024 14:03:24 +0100
Subject: [PATCH v19.1 1/2] Rename RowCompareType to CompareType
RowCompareType served as a way to describe the fundamental meaning of
an operator, notionally independent of an operator class (although so
far this was only really supported for btrees). Its original purpose
was for use inside RowCompareExpr, and it has also found some small
use outside, such as for get_op_btree_interpretation().
We want to expand this now, as a more general way to describe operator
semantics for other index access methods, including gist (to improve
GistTranslateStratnum()) and others not written yet. To avoid future
confusion, we rename the type to CompareType and the symbols from
ROWCOMPARE_XXX to COMPARE_XXX to reflect their more general purpose.
---
src/backend/executor/execExpr.c | 2 +-
src/backend/executor/execExprInterp.c | 12 ++++----
src/backend/executor/nodeIndexscan.c | 4 +--
src/backend/jit/llvm/llvmjit_expr.c | 12 ++++----
src/backend/optimizer/path/indxpath.c | 2 +-
src/backend/optimizer/util/predtest.c | 2 +-
src/backend/parser/parse_expr.c | 12 ++++----
src/backend/utils/cache/lsyscache.c | 6 ++--
src/include/executor/execExpr.h | 2 +-
src/include/nodes/primnodes.h | 41 ++++++++++++++++++---------
src/tools/pgindent/typedefs.list | 2 +-
11 files changed, 55 insertions(+), 42 deletions(-)
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index e0eb96fd5ad..afa0f6b14a6 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -2102,7 +2102,7 @@ ExecInitExprRec(Expr *node, ExprState *state,
/* Finally, examine the last comparison result */
scratch.opcode = EEOP_ROWCOMPARE_FINAL;
- scratch.d.rowcompare_final.rctype = rcexpr->rctype;
+ scratch.d.rowcompare_final.cmptype = rcexpr->cmptype;
ExprEvalPushStep(state, &scratch);
/* adjust jump targets */
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index bad7b195bfb..7ffb611ab8c 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -1438,22 +1438,22 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
EEO_CASE(EEOP_ROWCOMPARE_FINAL)
{
int32 cmpresult = DatumGetInt32(*op->resvalue);
- RowCompareType rctype = op->d.rowcompare_final.rctype;
+ CompareType cmptype = op->d.rowcompare_final.cmptype;
*op->resnull = false;
- switch (rctype)
+ switch (cmptype)
{
/* EQ and NE cases aren't allowed here */
- case ROWCOMPARE_LT:
+ case COMPARE_LT:
*op->resvalue = BoolGetDatum(cmpresult < 0);
break;
- case ROWCOMPARE_LE:
+ case COMPARE_LE:
*op->resvalue = BoolGetDatum(cmpresult <= 0);
break;
- case ROWCOMPARE_GE:
+ case COMPARE_GE:
*op->resvalue = BoolGetDatum(cmpresult >= 0);
break;
- case ROWCOMPARE_GT:
+ case COMPARE_GT:
*op->resvalue = BoolGetDatum(cmpresult > 0);
break;
default:
diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index 8000feff4c9..5377ce4ec30 100644
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -1344,7 +1344,7 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index,
&op_lefttype,
&op_righttype);
- if (op_strategy != rc->rctype)
+ if (op_strategy != rc->cmptype)
elog(ERROR, "RowCompare index qualification contains wrong operator");
opfuncid = get_opfamily_proc(opfamily,
@@ -1421,7 +1421,7 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index,
MemSet(this_scan_key, 0, sizeof(ScanKeyData));
this_scan_key->sk_flags = SK_ROW_HEADER;
this_scan_key->sk_attno = first_sub_key->sk_attno;
- this_scan_key->sk_strategy = rc->rctype;
+ this_scan_key->sk_strategy = rc->cmptype;
/* sk_subtype, sk_collation, sk_func not used in a header */
this_scan_key->sk_argument = PointerGetDatum(first_sub_key);
}
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index c533f552540..88bef871413 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -1770,7 +1770,7 @@ llvm_compile_expr(ExprState *state)
case EEOP_ROWCOMPARE_FINAL:
{
- RowCompareType rctype = op->d.rowcompare_final.rctype;
+ CompareType cmptype = op->d.rowcompare_final.cmptype;
LLVMValueRef v_cmpresult;
LLVMValueRef v_result;
@@ -1786,18 +1786,18 @@ llvm_compile_expr(ExprState *state)
l_load(b, TypeSizeT, v_resvaluep, ""),
LLVMInt32TypeInContext(lc), "");
- switch (rctype)
+ switch (cmptype)
{
- case ROWCOMPARE_LT:
+ case COMPARE_LT:
predicate = LLVMIntSLT;
break;
- case ROWCOMPARE_LE:
+ case COMPARE_LE:
predicate = LLVMIntSLE;
break;
- case ROWCOMPARE_GT:
+ case COMPARE_GT:
predicate = LLVMIntSGT;
break;
- case ROWCOMPARE_GE:
+ case COMPARE_GE:
predicate = LLVMIntSGE;
break;
default:
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 5d102a0d371..10f4c6a1f19 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -3675,7 +3675,7 @@ expand_indexqual_rowcompare(PlannerInfo *root,
{
RowCompareExpr *rc = makeNode(RowCompareExpr);
- rc->rctype = (RowCompareType) op_strategy;
+ rc->cmptype = (CompareType) op_strategy;
rc->opnos = new_ops;
rc->opfamilies = list_copy_head(clause->opfamilies,
matching_cols);
diff --git a/src/backend/optimizer/util/predtest.c b/src/backend/optimizer/util/predtest.c
index 0a132610140..e967d831bf1 100644
--- a/src/backend/optimizer/util/predtest.c
+++ b/src/backend/optimizer/util/predtest.c
@@ -1664,7 +1664,7 @@ clause_is_strict_for(Node *clause, Node *subexpr, bool allow_false)
#define BTEQ BTEqualStrategyNumber
#define BTGE BTGreaterEqualStrategyNumber
#define BTGT BTGreaterStrategyNumber
-#define BTNE ROWCOMPARE_NE
+#define BTNE COMPARE_NE
/* We use "none" for 0/false to make the tables align nicely */
#define none 0
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index c2806297aa4..f31e9a4e879 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -2807,7 +2807,7 @@ make_row_comparison_op(ParseState *pstate, List *opname,
List *largs, List *rargs, int location)
{
RowCompareExpr *rcexpr;
- RowCompareType rctype;
+ CompareType cmptype;
List *opexprs;
List *opnos;
List *opfamilies;
@@ -2928,15 +2928,15 @@ make_row_comparison_op(ParseState *pstate, List *opname,
errhint("Row comparison operators must be associated with btree operator families."),
parser_errposition(pstate, location)));
}
- rctype = (RowCompareType) i;
+ cmptype = (CompareType) i;
/*
* For = and <> cases, we just combine the pairwise operators with AND or
* OR respectively.
*/
- if (rctype == ROWCOMPARE_EQ)
+ if (cmptype == COMPARE_EQ)
return (Node *) makeBoolExpr(AND_EXPR, opexprs, location);
- if (rctype == ROWCOMPARE_NE)
+ if (cmptype == COMPARE_NE)
return (Node *) makeBoolExpr(OR_EXPR, opexprs, location);
/*
@@ -2953,7 +2953,7 @@ make_row_comparison_op(ParseState *pstate, List *opname,
{
OpBtreeInterpretation *opinfo = lfirst(j);
- if (opinfo->strategy == rctype)
+ if (opinfo->strategy == cmptype)
{
opfamily = opinfo->opfamily_id;
break;
@@ -2989,7 +2989,7 @@ make_row_comparison_op(ParseState *pstate, List *opname,
}
rcexpr = makeNode(RowCompareExpr);
- rcexpr->rctype = rctype;
+ rcexpr->cmptype = cmptype;
rcexpr->opnos = opnos;
rcexpr->opfamilies = opfamilies;
rcexpr->inputcollids = NIL; /* assign_expr_collations will fix this */
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index a85dc0d891f..af68dce0907 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -595,7 +595,7 @@ get_op_hash_functions(Oid opno,
*
* In addition to the normal btree operators, we consider a <> operator to be
* a "member" of an opfamily if its negator is an equality operator of the
- * opfamily. ROWCOMPARE_NE is returned as the strategy number for this case.
+ * opfamily. COMPARE_NE is returned as the strategy number for this case.
*/
List *
get_op_btree_interpretation(Oid opno)
@@ -666,11 +666,11 @@ get_op_btree_interpretation(Oid opno)
if (op_strategy != BTEqualStrategyNumber)
continue;
- /* OK, report it with "strategy" ROWCOMPARE_NE */
+ /* OK, report it with "strategy" COMPARE_NE */
thisresult = (OpBtreeInterpretation *)
palloc(sizeof(OpBtreeInterpretation));
thisresult->opfamily_id = op_form->amopfamily;
- thisresult->strategy = ROWCOMPARE_NE;
+ thisresult->strategy = COMPARE_NE;
thisresult->oplefttype = op_form->amoplefttype;
thisresult->oprighttype = op_form->amoprighttype;
result = lappend(result, thisresult);
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index 56fb0d0adbe..42db1161da1 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -494,7 +494,7 @@ typedef struct ExprEvalStep
/* for EEOP_ROWCOMPARE_FINAL */
struct
{
- RowCompareType rctype;
+ CompareType cmptype;
} rowcompare_final;
/* for EEOP_MINMAX */
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index b0ef1952e8f..107eb36a340 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1435,6 +1435,31 @@ typedef struct RowExpr
ParseLoc location; /* token location, or -1 if unknown */
} RowExpr;
+/*
+ * CompareType - fundamental semantics of certain operators
+ *
+ * These enum symbols represent the fundamental semantics of certain operators
+ * that the system needs to have some hardcoded knowledge about. (For
+ * example, RowCompareExpr needs to know which operators can be determined to
+ * act like =, <>, <, etc.) Index access methods map (some of) strategy
+ * numbers to these values so that the system can know about the meaning of
+ * (some of) the operators without needing hardcoded knowledge of index AM's
+ * strategy numbering.
+ *
+ * XXX Currently, this mapping is not fully developed and the values are
+ * chosen to match btree strategy numbers, which is not going to work very
+ * well for other access methods.
+ */
+typedef enum CompareType
+{
+ COMPARE_LT = 1, /* BTLessStrategyNumber */
+ COMPARE_LE = 2, /* BTLessEqualStrategyNumber */
+ COMPARE_EQ = 3, /* BTEqualStrategyNumber */
+ COMPARE_GE = 4, /* BTGreaterEqualStrategyNumber */
+ COMPARE_GT = 5, /* BTGreaterStrategyNumber */
+ COMPARE_NE = 6, /* no such btree strategy */
+} CompareType;
+
/*
* RowCompareExpr - row-wise comparison, such as (a, b) <= (1, 2)
*
@@ -1446,26 +1471,14 @@ typedef struct RowExpr
*
* A RowCompareExpr node is only generated for the < <= > >= cases;
* the = and <> cases are translated to simple AND or OR combinations
- * of the pairwise comparisons. However, we include = and <> in the
- * RowCompareType enum for the convenience of parser logic.
+ * of the pairwise comparisons.
*/
-typedef enum RowCompareType
-{
- /* Values of this enum are chosen to match btree strategy numbers */
- ROWCOMPARE_LT = 1, /* BTLessStrategyNumber */
- ROWCOMPARE_LE = 2, /* BTLessEqualStrategyNumber */
- ROWCOMPARE_EQ = 3, /* BTEqualStrategyNumber */
- ROWCOMPARE_GE = 4, /* BTGreaterEqualStrategyNumber */
- ROWCOMPARE_GT = 5, /* BTGreaterStrategyNumber */
- ROWCOMPARE_NE = 6, /* no such btree strategy */
-} RowCompareType;
-
typedef struct RowCompareExpr
{
Expr xpr;
/* LT LE GE or GT, never EQ or NE */
- RowCompareType rctype;
+ CompareType cmptype;
/* OID list of pairwise comparison ops */
List *opnos pg_node_attr(query_jumble_ignore);
/* OID list of containing operator families */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 2d4c870423a..4100e634964 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -455,6 +455,7 @@ CommitTimestampShared
CommonEntry
CommonTableExpr
CompareScalarsContext
+CompareType
CompiledExprState
CompositeIOData
CompositeTypeStmt
@@ -2487,7 +2488,6 @@ RoleSpecType
RoleStmtType
RollupData
RowCompareExpr
-RowCompareType
RowExpr
RowIdentityVarInfo
RowMarkClause
base-commit: 7727049e8f663344d4d0457e1d9ec048d626f3d9
--
2.47.1
[text/plain] v19.1-0002-Change-gist-stratnum-function-to-use-CompareTy.patch (30.2K, ../../[email protected]/3-v19.1-0002-Change-gist-stratnum-function-to-use-CompareTy.patch)
download | inline diff:
From 9509de62880094562eb234e4bdecbaed5062ff91 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 4 Dec 2024 15:31:18 +0100
Subject: [PATCH v19.1 2/2] Change gist stratnum function to use CompareType
This changes commit 7406ab623fe in that the gist strategy number
mapping support function is changed to use the CompareType enum as
input, instead of the "well-known" RT*StrategyNumber strategy numbers.
This is a bit cleaner, since you are not dealing with two sets of
strategy numbers. Also, this will enable us to subsume this system
into a more general system of using CompareType to define operator
semantics across index methods.
---
contrib/btree_gist/btree_gist--1.7--1.8.sql | 54 ++++++++++----------
contrib/btree_gist/btree_gist.c | 15 +++---
contrib/btree_gist/expected/stratnum.out | 4 +-
contrib/btree_gist/sql/stratnum.sql | 4 +-
doc/src/sgml/gist.sgml | 24 ++++-----
doc/src/sgml/xindex.sgml | 2 +-
src/backend/access/gist/gistutil.c | 35 +++++++++----
src/backend/access/gist/gistvalidate.c | 2 +-
src/backend/catalog/pg_constraint.c | 20 ++++----
src/backend/commands/indexcmds.c | 50 +++++++++---------
src/backend/commands/tablecmds.c | 10 ++--
src/include/access/gist.h | 3 +-
src/include/catalog/pg_amproc.dat | 12 ++---
src/include/catalog/pg_proc.dat | 6 +--
src/include/commands/defrem.h | 4 +-
src/include/nodes/primnodes.h | 4 +-
src/test/regress/expected/misc_functions.out | 16 +++---
src/test/regress/sql/misc_functions.sql | 4 +-
18 files changed, 144 insertions(+), 125 deletions(-)
diff --git a/contrib/btree_gist/btree_gist--1.7--1.8.sql b/contrib/btree_gist/btree_gist--1.7--1.8.sql
index 307bfe574b0..c702426deab 100644
--- a/contrib/btree_gist/btree_gist--1.7--1.8.sql
+++ b/contrib/btree_gist/btree_gist--1.7--1.8.sql
@@ -3,85 +3,85 @@
-- complain if script is sourced in psql, rather than via CREATE EXTENSION
\echo Use "ALTER EXTENSION btree_gist UPDATE TO '1.8'" to load this file. \quit
-CREATE FUNCTION gist_stratnum_btree(smallint)
+CREATE FUNCTION gist_stratnum_btree(int)
RETURNS smallint
AS 'MODULE_PATHNAME'
LANGUAGE C IMMUTABLE PARALLEL SAFE STRICT;
ALTER OPERATOR FAMILY gist_oid_ops USING gist ADD
- FUNCTION 12 (oid, oid) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (oid, oid) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_int2_ops USING gist ADD
- FUNCTION 12 (int2, int2) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (int2, int2) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_int4_ops USING gist ADD
- FUNCTION 12 (int4, int4) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (int4, int4) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_int8_ops USING gist ADD
- FUNCTION 12 (int8, int8) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (int8, int8) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_float4_ops USING gist ADD
- FUNCTION 12 (float4, float4) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (float4, float4) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_float8_ops USING gist ADD
- FUNCTION 12 (float8, float8) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (float8, float8) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_timestamp_ops USING gist ADD
- FUNCTION 12 (timestamp, timestamp) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (timestamp, timestamp) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_timestamptz_ops USING gist ADD
- FUNCTION 12 (timestamptz, timestamptz) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (timestamptz, timestamptz) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_time_ops USING gist ADD
- FUNCTION 12 (time, time) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (time, time) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_date_ops USING gist ADD
- FUNCTION 12 (date, date) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (date, date) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_interval_ops USING gist ADD
- FUNCTION 12 (interval, interval) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (interval, interval) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_cash_ops USING gist ADD
- FUNCTION 12 (money, money) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (money, money) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_macaddr_ops USING gist ADD
- FUNCTION 12 (macaddr, macaddr) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (macaddr, macaddr) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_text_ops USING gist ADD
- FUNCTION 12 (text, text) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (text, text) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_bpchar_ops USING gist ADD
- FUNCTION 12 (bpchar, bpchar) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (bpchar, bpchar) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_bytea_ops USING gist ADD
- FUNCTION 12 (bytea, bytea) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (bytea, bytea) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_numeric_ops USING gist ADD
- FUNCTION 12 (numeric, numeric) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (numeric, numeric) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_bit_ops USING gist ADD
- FUNCTION 12 (bit, bit) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (bit, bit) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_vbit_ops USING gist ADD
- FUNCTION 12 (varbit, varbit) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (varbit, varbit) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_inet_ops USING gist ADD
- FUNCTION 12 (inet, inet) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (inet, inet) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_cidr_ops USING gist ADD
- FUNCTION 12 (cidr, cidr) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (cidr, cidr) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_timetz_ops USING gist ADD
- FUNCTION 12 (timetz, timetz) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (timetz, timetz) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_uuid_ops USING gist ADD
- FUNCTION 12 (uuid, uuid) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (uuid, uuid) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_macaddr8_ops USING gist ADD
- FUNCTION 12 (macaddr8, macaddr8) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (macaddr8, macaddr8) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_enum_ops USING gist ADD
- FUNCTION 12 (anyenum, anyenum) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (anyenum, anyenum) gist_stratnum_btree (int) ;
ALTER OPERATOR FAMILY gist_bool_ops USING gist ADD
- FUNCTION 12 (bool, bool) gist_stratnum_btree (int2) ;
+ FUNCTION 12 (bool, bool) gist_stratnum_btree (int) ;
diff --git a/contrib/btree_gist/btree_gist.c b/contrib/btree_gist/btree_gist.c
index 5fd4cce27d0..fc6c7091795 100644
--- a/contrib/btree_gist/btree_gist.c
+++ b/contrib/btree_gist/btree_gist.c
@@ -4,6 +4,7 @@
#include "postgres.h"
#include "access/stratnum.h"
+#include "nodes/primnodes.h"
#include "utils/builtins.h"
PG_MODULE_MAGIC;
@@ -60,19 +61,19 @@ gbt_decompress(PG_FUNCTION_ARGS)
Datum
gist_stratnum_btree(PG_FUNCTION_ARGS)
{
- StrategyNumber strat = PG_GETARG_UINT16(0);
+ CompareType cmptype = PG_GETARG_INT32(0);
- switch (strat)
+ switch (cmptype)
{
- case RTEqualStrategyNumber:
+ case COMPARE_EQ:
PG_RETURN_UINT16(BTEqualStrategyNumber);
- case RTLessStrategyNumber:
+ case COMPARE_LT:
PG_RETURN_UINT16(BTLessStrategyNumber);
- case RTLessEqualStrategyNumber:
+ case COMPARE_LE:
PG_RETURN_UINT16(BTLessEqualStrategyNumber);
- case RTGreaterStrategyNumber:
+ case COMPARE_GT:
PG_RETURN_UINT16(BTGreaterStrategyNumber);
- case RTGreaterEqualStrategyNumber:
+ case COMPARE_GE:
PG_RETURN_UINT16(BTGreaterEqualStrategyNumber);
default:
PG_RETURN_UINT16(InvalidStrategy);
diff --git a/contrib/btree_gist/expected/stratnum.out b/contrib/btree_gist/expected/stratnum.out
index 9d80c6590d9..dd0edaf4a20 100644
--- a/contrib/btree_gist/expected/stratnum.out
+++ b/contrib/btree_gist/expected/stratnum.out
@@ -1,11 +1,11 @@
-- test stratnum support func
-SELECT gist_stratnum_btree(3::smallint);
+SELECT gist_stratnum_btree(7);
gist_stratnum_btree
---------------------
0
(1 row)
-SELECT gist_stratnum_btree(18::smallint);
+SELECT gist_stratnum_btree(3);
gist_stratnum_btree
---------------------
3
diff --git a/contrib/btree_gist/sql/stratnum.sql b/contrib/btree_gist/sql/stratnum.sql
index f58cdbe93da..75adddad849 100644
--- a/contrib/btree_gist/sql/stratnum.sql
+++ b/contrib/btree_gist/sql/stratnum.sql
@@ -1,3 +1,3 @@
-- test stratnum support func
-SELECT gist_stratnum_btree(3::smallint);
-SELECT gist_stratnum_btree(18::smallint);
+SELECT gist_stratnum_btree(7);
+SELECT gist_stratnum_btree(3);
diff --git a/doc/src/sgml/gist.sgml b/doc/src/sgml/gist.sgml
index 638d912dc2d..99098ab2522 100644
--- a/doc/src/sgml/gist.sgml
+++ b/doc/src/sgml/gist.sgml
@@ -290,8 +290,8 @@ <title>Extensibility</title>
The optional eleventh method <function>sortsupport</function> is used to
speed up building a <acronym>GiST</acronym> index.
The optional twelfth method <function>stratnum</function> is used to
- translate well-known <literal>RT*StrategyNumber</literal>s (from
- <filename>src/include/access/stratnum.h</filename>) into strategy numbers
+ translate compare types (from
+ <filename>src/include/nodes/primnodes.h</filename>) into strategy numbers
used by the operator class. This lets the core code look up operators for
temporal constraint indexes.
</para>
@@ -1173,8 +1173,8 @@ <title>Extensibility</title>
<term><function>stratnum</function></term>
<listitem>
<para>
- Given an <literal>RT*StrategyNumber</literal> value from
- <filename>src/include/access/stratnum.h</filename>, returns a strategy
+ Given a <literal>CompareType</literal> value from
+ <filename>src/include/nodes/primnodes.h</filename>, returns a strategy
number used by this operator class for matching functionality. The
function should return <literal>InvalidStrategy</literal> if the
operator class has no matching strategy.
@@ -1184,7 +1184,7 @@ <title>Extensibility</title>
This is used for temporal index constraints (i.e., <literal>PRIMARY
KEY</literal> and <literal>UNIQUE</literal>). If the operator class
provides this function and it returns results for
- <literal>RTEqualStrategyNumber</literal>, it can be used in the
+ <literal>COMPARE_EQ</literal>, it can be used in the
non-<literal>WITHOUT OVERLAPS</literal> part(s) of an index constraint.
</para>
@@ -1194,7 +1194,7 @@ <title>Extensibility</title>
<programlisting>
CREATE OR REPLACE FUNCTION my_stratnum(integer)
-RETURNS integer
+RETURNS smallint
AS 'MODULE_PATHNAME'
LANGUAGE C STRICT;
</programlisting>
@@ -1209,12 +1209,12 @@ <title>Extensibility</title>
Datum
my_stratnum(PG_FUNCTION_ARGS)
{
- StrategyNumber strategy = (StrategyNumber) PG_GETARG_UINT16(1);
+ CompareType cmptype = PG_GETARG_INT32(0);
StrategyNumber ret = InvalidStrategy;
- switch (strategy)
+ switch (cmptype)
{
- case RTEqualStrategyNumber:
+ case COMPARE_EQ:
ret = BTEqualStrategyNumber;
}
@@ -1226,9 +1226,9 @@ <title>Extensibility</title>
<para>
One translation function is provided by
<productname>PostgreSQL</productname>:
- <literal>gist_stratnum_identity</literal> is for operator classes that
- already use the <literal>RT*StrategyNumber</literal> constants. It
- returns whatever is passed to it. The <literal>btree_gist</literal>
+ <literal>gist_stratnum_common</literal> is for operator classes that
+ use the <literal>RT*StrategyNumber</literal> constants.
+ The <literal>btree_gist</literal>
extension defines a second translation function,
<literal>gist_stratnum_btree</literal>, for operator classes that use
the <literal>BT*StrategyNumber</literal> constants.
diff --git a/doc/src/sgml/xindex.sgml b/doc/src/sgml/xindex.sgml
index 3a19dab15e0..05361962495 100644
--- a/doc/src/sgml/xindex.sgml
+++ b/doc/src/sgml/xindex.sgml
@@ -592,7 +592,7 @@ <title>GiST Support Functions</title>
</row>
<row>
<entry><function>stratnum</function></entry>
- <entry>translate well-known strategy numbers to ones
+ <entry>translate compare types to strategy numbers
used by the operator class (optional)</entry>
<entry>12</entry>
</row>
diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c
index d2d0b36d4ea..bde541a6251 100644
--- a/src/backend/access/gist/gistutil.c
+++ b/src/backend/access/gist/gistutil.c
@@ -1058,27 +1058,44 @@ gistGetFakeLSN(Relation rel)
}
/*
- * Returns the same number that was received.
- *
- * This is for GiST opclasses that use the RT*StrategyNumber constants.
+ * This is a stratnum support function for GiST opclasses that use the
+ * RT*StrategyNumber constants.
*/
Datum
-gist_stratnum_identity(PG_FUNCTION_ARGS)
+gist_stratnum_common(PG_FUNCTION_ARGS)
{
- StrategyNumber strat = PG_GETARG_UINT16(0);
+ CompareType cmptype = PG_GETARG_INT32(0);
- PG_RETURN_UINT16(strat);
+ switch (cmptype)
+ {
+ case COMPARE_EQ:
+ PG_RETURN_UINT16(RTEqualStrategyNumber);
+ case COMPARE_LT:
+ PG_RETURN_UINT16(RTLessStrategyNumber);
+ case COMPARE_LE:
+ PG_RETURN_UINT16(RTLessEqualStrategyNumber);
+ case COMPARE_GT:
+ PG_RETURN_UINT16(RTGreaterStrategyNumber);
+ case COMPARE_GE:
+ PG_RETURN_UINT16(RTGreaterEqualStrategyNumber);
+ case COMPARE_OVERLAP:
+ PG_RETURN_UINT16(RTOverlapStrategyNumber);
+ case COMPARE_CONTAINED_BY:
+ PG_RETURN_UINT16(RTContainedByStrategyNumber);
+ default:
+ PG_RETURN_UINT16(InvalidStrategy);
+ }
}
/*
- * Returns the opclass's private stratnum used for the given strategy.
+ * Returns the opclass's private stratnum used for the given compare type.
*
* Calls the opclass's GIST_STRATNUM_PROC support function, if any,
* and returns the result.
* Returns InvalidStrategy if the function is not defined.
*/
StrategyNumber
-GistTranslateStratnum(Oid opclass, StrategyNumber strat)
+GistTranslateStratnum(Oid opclass, CompareType cmptype)
{
Oid opfamily;
Oid opcintype;
@@ -1095,6 +1112,6 @@ GistTranslateStratnum(Oid opclass, StrategyNumber strat)
return InvalidStrategy;
/* Ask the translation function */
- result = OidFunctionCall1Coll(funcid, InvalidOid, UInt16GetDatum(strat));
+ result = OidFunctionCall1Coll(funcid, InvalidOid, Int32GetDatum(cmptype));
return DatumGetUInt16(result);
}
diff --git a/src/backend/access/gist/gistvalidate.c b/src/backend/access/gist/gistvalidate.c
index 0901543a60a..7d51fc167d6 100644
--- a/src/backend/access/gist/gistvalidate.c
+++ b/src/backend/access/gist/gistvalidate.c
@@ -148,7 +148,7 @@ gistvalidate(Oid opclassoid)
break;
case GIST_STRATNUM_PROC:
ok = check_amproc_signature(procform->amproc, INT2OID, true,
- 1, 1, INT2OID);
+ 1, 1, INT4OID);
break;
default:
ereport(INFO,
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 9c05a98d28c..4a076e2874e 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -1638,22 +1638,22 @@ FindFKPeriodOpers(Oid opclass,
* of the old value, then we can treat the attribute as if it didn't
* change, and skip the RI check.
*/
- strat = RTContainedByStrategyNumber;
- GetOperatorFromWellKnownStrategy(opclass,
- InvalidOid,
- containedbyoperoid,
- &strat);
+ GetOperatorFromCompareType(opclass,
+ InvalidOid,
+ COMPARE_CONTAINED_BY,
+ containedbyoperoid,
+ &strat);
/*
* Now look up the ContainedBy operator. Its left arg must be the type of
* the column (or rather of the opclass). Its right arg must match the
* return type of the support proc.
*/
- strat = RTContainedByStrategyNumber;
- GetOperatorFromWellKnownStrategy(opclass,
- ANYMULTIRANGEOID,
- aggedcontainedbyoperoid,
- &strat);
+ GetOperatorFromCompareType(opclass,
+ ANYMULTIRANGEOID,
+ COMPARE_CONTAINED_BY,
+ aggedcontainedbyoperoid,
+ &strat);
}
/*
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 4049ce1a10f..9c1a159754c 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2178,15 +2178,15 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
}
else if (iswithoutoverlaps)
{
+ CompareType cmptype;
StrategyNumber strat;
Oid opid;
if (attn == nkeycols - 1)
- strat = RTOverlapStrategyNumber;
+ cmptype = COMPARE_OVERLAP;
else
- strat = RTEqualStrategyNumber;
- GetOperatorFromWellKnownStrategy(opclassOids[attn], InvalidOid,
- &opid, &strat);
+ cmptype = COMPARE_EQ;
+ GetOperatorFromCompareType(opclassOids[attn], InvalidOid, cmptype, &opid, &strat);
indexInfo->ii_ExclusionOps[attn] = opid;
indexInfo->ii_ExclusionProcs[attn] = get_opcode(opid);
indexInfo->ii_ExclusionStrats[attn] = strat;
@@ -2422,30 +2422,28 @@ GetDefaultOpClass(Oid type_id, Oid am_id)
}
/*
- * GetOperatorFromWellKnownStrategy
+ * GetOperatorFromCompareType
*
* opclass - the opclass to use
* rhstype - the type for the right-hand side, or InvalidOid to use the type of the given opclass.
+ * cmptype - kind of operator to find
* opid - holds the operator we found
- * strat - holds the input and output strategy number
+ * strat - holds the output strategy number
*
- * Finds an operator from a "well-known" strategy number. This is used for
- * temporal index constraints (and other temporal features) to look up
- * equality and overlaps operators, since the strategy numbers for non-btree
- * indexams need not follow any fixed scheme. We ask an opclass support
- * function to translate from the well-known number to the internal value. If
- * the function isn't defined or it gives no result, we return
- * InvalidStrategy.
+ * Finds an operator from a CompareType. This is used for temporal index
+ * constraints (and other temporal features) to look up equality and overlaps
+ * operators. We ask an opclass support function to translate from the
+ * compare type to the internal strategy numbers. If the function isn't
+ * defined or it gives no result, we set *strat to InvalidStrategy.
*/
void
-GetOperatorFromWellKnownStrategy(Oid opclass, Oid rhstype,
- Oid *opid, StrategyNumber *strat)
+GetOperatorFromCompareType(Oid opclass, Oid rhstype, CompareType cmptype,
+ Oid *opid, StrategyNumber *strat)
{
Oid opfamily;
Oid opcintype;
- StrategyNumber instrat = *strat;
- Assert(instrat == RTEqualStrategyNumber || instrat == RTOverlapStrategyNumber || instrat == RTContainedByStrategyNumber);
+ Assert(cmptype == COMPARE_EQ || cmptype == COMPARE_OVERLAP || cmptype == COMPARE_CONTAINED_BY);
*opid = InvalidOid;
@@ -2457,7 +2455,7 @@ GetOperatorFromWellKnownStrategy(Oid opclass, Oid rhstype,
* For now we only need GiST support, but this could support other
* indexams if we wanted.
*/
- *strat = GistTranslateStratnum(opclass, instrat);
+ *strat = GistTranslateStratnum(opclass, cmptype);
if (*strat == InvalidStrategy)
{
HeapTuple tuple;
@@ -2468,11 +2466,11 @@ GetOperatorFromWellKnownStrategy(Oid opclass, Oid rhstype,
ereport(ERROR,
errcode(ERRCODE_UNDEFINED_OBJECT),
- instrat == RTEqualStrategyNumber ? errmsg("could not identify an equality operator for type %s", format_type_be(opcintype)) :
- instrat == RTOverlapStrategyNumber ? errmsg("could not identify an overlaps operator for type %s", format_type_be(opcintype)) :
- instrat == RTContainedByStrategyNumber ? errmsg("could not identify a contained-by operator for type %s", format_type_be(opcintype)) : 0,
- errdetail("Could not translate strategy number %d for operator class \"%s\" for access method \"%s\".",
- instrat, NameStr(((Form_pg_opclass) GETSTRUCT(tuple))->opcname), "gist"));
+ cmptype = COMPARE_EQ ? errmsg("could not identify an equality operator for type %s", format_type_be(opcintype)) :
+ cmptype == COMPARE_OVERLAP ? errmsg("could not identify an overlaps operator for type %s", format_type_be(opcintype)) :
+ cmptype == COMPARE_CONTAINED_BY ? errmsg("could not identify a contained-by operator for type %s", format_type_be(opcintype)) : 0,
+ errdetail("Could not translate compare type %d for operator class \"%s\" for access method \"%s\".",
+ cmptype, NameStr(((Form_pg_opclass) GETSTRUCT(tuple))->opcname), "gist"));
}
/*
@@ -2495,9 +2493,9 @@ GetOperatorFromWellKnownStrategy(Oid opclass, Oid rhstype,
ereport(ERROR,
errcode(ERRCODE_UNDEFINED_OBJECT),
- instrat == RTEqualStrategyNumber ? errmsg("could not identify an equality operator for type %s", format_type_be(opcintype)) :
- instrat == RTOverlapStrategyNumber ? errmsg("could not identify an overlaps operator for type %s", format_type_be(opcintype)) :
- instrat == RTContainedByStrategyNumber ? errmsg("could not identify a contained-by operator for type %s", format_type_be(opcintype)) : 0,
+ cmptype == COMPARE_EQ ? errmsg("could not identify an equality operator for type %s", format_type_be(opcintype)) :
+ cmptype == COMPARE_OVERLAP ? errmsg("could not identify an overlaps operator for type %s", format_type_be(opcintype)) :
+ cmptype == COMPARE_CONTAINED_BY ? errmsg("could not identify a contained-by operator for type %s", format_type_be(opcintype)) : 0,
errdetail("There is no suitable operator in operator family \"%s\" for access method \"%s\".",
NameStr(((Form_pg_opfamily) GETSTRUCT(tuple))->opfname), "gist"));
}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 6ccae4cb4a8..aef672d278d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9972,7 +9972,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
if (with_period)
{
- StrategyNumber rtstrategy;
+ CompareType cmptype;
bool for_overlaps = with_period && i == numpks - 1;
/*
@@ -9982,14 +9982,14 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
if (amid != GIST_AM_OID)
elog(ERROR, "only GiST indexes are supported for temporal foreign keys");
- rtstrategy = for_overlaps ? RTOverlapStrategyNumber : RTEqualStrategyNumber;
+ cmptype = for_overlaps ? COMPARE_OVERLAP : COMPARE_EQ;
/*
* An opclass can use whatever strategy numbers it wants, so we
* ask the opclass what number it actually uses instead of our RT*
* constants.
*/
- eqstrategy = GistTranslateStratnum(opclasses[i], rtstrategy);
+ eqstrategy = GistTranslateStratnum(opclasses[i], cmptype);
if (eqstrategy == InvalidStrategy)
{
HeapTuple tuple;
@@ -10003,8 +10003,8 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
for_overlaps
? errmsg("could not identify an overlaps operator for foreign key")
: errmsg("could not identify an equality operator for foreign key"),
- errdetail("Could not translate strategy number %d for operator class \"%s\" for access method \"%s\".",
- rtstrategy, NameStr(((Form_pg_opclass) GETSTRUCT(tuple))->opcname), "gist"));
+ errdetail("Could not translate compare type %d for operator class \"%s\" for access method \"%s\".",
+ cmptype, NameStr(((Form_pg_opclass) GETSTRUCT(tuple))->opcname), "gist"));
}
}
else
diff --git a/src/include/access/gist.h b/src/include/access/gist.h
index 22dd04c1418..a8689cfcefe 100644
--- a/src/include/access/gist.h
+++ b/src/include/access/gist.h
@@ -247,6 +247,7 @@ typedef struct
do { (e).key = (k); (e).rel = (r); (e).page = (pg); \
(e).offset = (o); (e).leafkey = (l); } while (0)
-extern StrategyNumber GistTranslateStratnum(Oid opclass, StrategyNumber strat);
+enum CompareType;
+extern StrategyNumber GistTranslateStratnum(Oid opclass, enum CompareType cmp);
#endif /* GIST_H */
diff --git a/src/include/catalog/pg_amproc.dat b/src/include/catalog/pg_amproc.dat
index 5d7fe292bf6..a04828fde96 100644
--- a/src/include/catalog/pg_amproc.dat
+++ b/src/include/catalog/pg_amproc.dat
@@ -508,7 +508,7 @@
amprocrighttype => 'box', amprocnum => '8', amproc => 'gist_box_distance' },
{ amprocfamily => 'gist/box_ops', amproclefttype => 'box',
amprocrighttype => 'box', amprocnum => '12',
- amproc => 'gist_stratnum_identity' },
+ amproc => 'gist_stratnum_common' },
{ amprocfamily => 'gist/poly_ops', amproclefttype => 'polygon',
amprocrighttype => 'polygon', amprocnum => '1',
amproc => 'gist_poly_consistent' },
@@ -530,7 +530,7 @@
amproc => 'gist_poly_distance' },
{ amprocfamily => 'gist/poly_ops', amproclefttype => 'polygon',
amprocrighttype => 'polygon', amprocnum => '12',
- amproc => 'gist_stratnum_identity' },
+ amproc => 'gist_stratnum_common' },
{ amprocfamily => 'gist/circle_ops', amproclefttype => 'circle',
amprocrighttype => 'circle', amprocnum => '1',
amproc => 'gist_circle_consistent' },
@@ -551,7 +551,7 @@
amproc => 'gist_circle_distance' },
{ amprocfamily => 'gist/circle_ops', amproclefttype => 'circle',
amprocrighttype => 'circle', amprocnum => '12',
- amproc => 'gist_stratnum_identity' },
+ amproc => 'gist_stratnum_common' },
{ amprocfamily => 'gist/tsvector_ops', amproclefttype => 'tsvector',
amprocrighttype => 'tsvector', amprocnum => '1',
amproc => 'gtsvector_consistent(internal,tsvector,int2,oid,internal)' },
@@ -608,7 +608,7 @@
amproc => 'range_gist_same' },
{ amprocfamily => 'gist/range_ops', amproclefttype => 'anyrange',
amprocrighttype => 'anyrange', amprocnum => '12',
- amproc => 'gist_stratnum_identity' },
+ amproc => 'gist_stratnum_common' },
{ amprocfamily => 'gist/network_ops', amproclefttype => 'inet',
amprocrighttype => 'inet', amprocnum => '1',
amproc => 'inet_gist_consistent' },
@@ -627,7 +627,7 @@
amprocrighttype => 'inet', amprocnum => '9', amproc => 'inet_gist_fetch' },
{ amprocfamily => 'gist/network_ops', amproclefttype => 'inet',
amprocrighttype => 'inet', amprocnum => '12',
- amproc => 'gist_stratnum_identity' },
+ amproc => 'gist_stratnum_common' },
{ amprocfamily => 'gist/multirange_ops', amproclefttype => 'anymultirange',
amprocrighttype => 'anymultirange', amprocnum => '1',
amproc => 'multirange_gist_consistent' },
@@ -648,7 +648,7 @@
amproc => 'range_gist_same' },
{ amprocfamily => 'gist/multirange_ops', amproclefttype => 'anymultirange',
amprocrighttype => 'anymultirange', amprocnum => '12',
- amproc => 'gist_stratnum_identity' },
+ amproc => 'gist_stratnum_common' },
# gin
{ amprocfamily => 'gin/array_ops', amproclefttype => 'anyarray',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 9575524007f..988b8a336f7 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -12403,8 +12403,8 @@
# GiST stratnum implementations
{ oid => '8047', descr => 'GiST support',
- proname => 'gist_stratnum_identity', prorettype => 'int2',
- proargtypes => 'int2',
- prosrc => 'gist_stratnum_identity' },
+ proname => 'gist_stratnum_common', prorettype => 'int2',
+ proargtypes => 'int4',
+ prosrc => 'gist_stratnum_common' },
]
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 5fd095ea177..bf44d111c00 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -50,8 +50,8 @@ extern bool CheckIndexCompatible(Oid oldId,
extern Oid GetDefaultOpClass(Oid type_id, Oid am_id);
extern Oid ResolveOpClass(const List *opclass, Oid attrType,
const char *accessMethodName, Oid accessMethodId);
-extern void GetOperatorFromWellKnownStrategy(Oid opclass, Oid rhstype,
- Oid *opid, StrategyNumber *strat);
+extern void GetOperatorFromCompareType(Oid opclass, Oid rhstype, CompareType cmptype,
+ Oid *opid, StrategyNumber *strat);
/* commands/functioncmds.c */
extern ObjectAddress CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt);
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 107eb36a340..60a8c62fca4 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1446,7 +1446,7 @@ typedef struct RowExpr
* (some of) the operators without needing hardcoded knowledge of index AM's
* strategy numbering.
*
- * XXX Currently, this mapping is not fully developed and the values are
+ * XXX Currently, this mapping is not fully developed and most values are
* chosen to match btree strategy numbers, which is not going to work very
* well for other access methods.
*/
@@ -1458,6 +1458,8 @@ typedef enum CompareType
COMPARE_GE = 4, /* BTGreaterEqualStrategyNumber */
COMPARE_GT = 5, /* BTGreaterStrategyNumber */
COMPARE_NE = 6, /* no such btree strategy */
+ COMPARE_OVERLAP,
+ COMPARE_CONTAINED_BY,
} CompareType;
/*
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 8c73d2b621e..dc28fcb310e 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -891,15 +891,15 @@ SELECT pg_column_toast_chunk_id(a) IS NULL,
DROP TABLE test_chunk_id;
DROP FUNCTION explain_mask_costs(text, bool, bool, bool, bool);
-- test stratnum support functions
-SELECT gist_stratnum_identity(3::smallint);
- gist_stratnum_identity
-------------------------
- 3
+SELECT gist_stratnum_common(7);
+ gist_stratnum_common
+----------------------
+ 3
(1 row)
-SELECT gist_stratnum_identity(18::smallint);
- gist_stratnum_identity
-------------------------
- 18
+SELECT gist_stratnum_common(3);
+ gist_stratnum_common
+----------------------
+ 18
(1 row)
diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql
index 0a0c6e5a6bb..81ef45d128c 100644
--- a/src/test/regress/sql/misc_functions.sql
+++ b/src/test/regress/sql/misc_functions.sql
@@ -401,5 +401,5 @@ CREATE TABLE test_chunk_id (a TEXT, b TEXT STORAGE EXTERNAL);
DROP FUNCTION explain_mask_costs(text, bool, bool, bool, bool);
-- test stratnum support functions
-SELECT gist_stratnum_identity(3::smallint);
-SELECT gist_stratnum_identity(18::smallint);
+SELECT gist_stratnum_common(7);
+SELECT gist_stratnum_common(3);
--
2.47.1
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Index AM API cleanup
@ 2025-01-05 23:20 Paul Jungwirth <[email protected]>
parent: Peter Eisentraut <[email protected]>
1 sibling, 0 replies; 14+ messages in thread
From: Paul Jungwirth @ 2025-01-05 23:20 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; Mark Dilger <[email protected]>; +Cc: pgsql-hackers; Kirill Reshke <[email protected]>; Andrew Dunstan <[email protected]>; Alex Wang <[email protected]>
On 8/26/24 08:10, Mark Dilger wrote:
> Paul, it seems what you are doing in v39-0001-Add-stratnum-GiST-support-function.patch is similar
to what I am doing in v17-0012-Convert-strategies-to-and-from-row-compare-types.patch.
Thank you inviting me to share some thoughts here! The goals of this patch make a lot of sense to
me. I'm very interested in making the non-btree AMs more useful, both to users and to Postgres core
itself. Here is a breakdown of what I think so far:
- Strategy Numbers
- Can you create unique GiST indexes or not?
- Updating opclasses
- Logical replication
- Empty ranges
Most of this is pretty general and not a line-by-line review of the patches. I'd like to do that
too, but this email is already long and late.
# Strategy Numbers
I think this is the latest re strategy numbers:
On 12/4/24 06:49, Peter Eisentraut wrote:
> On 27.11.24 13:57, Peter Eisentraut wrote:
>> I think, however, that we should rename RowCompareType. Otherwise, it's just going to be
>> confusing forevermore. I suggest to rename it simply to CompareType.
>
>> I'm going to try to code up the gist support on top of this patch set to make sure that it will
>> fit well. I'll report back.
>
> Here is a patch set in that direction. It renames RowCompareType to CompareType and updates the
> surrounding commentary a bit. And then I'm changing the gist strategy mapping to use the
> CompareType values instead of the RT* strategy numbers. Seeing this now, I like this a lot better
> than what we have now, because it makes it clearer in the API and the code what is a real strategy
> number and what's a different kind of thing. (This isn't entirely the above-mentioned integration
> of the gist support into your patch set yet, but it's a meaningful part of it.)
I see that this supports COMPARE_OVERLAPS and COMPARE_CONTAINED_BY, which provides most of what
foreign keys require. Great!
But I also need the intersect operator (*). There is no stratnum for that. Even if I added a
stratnum, it is not really about an *index*. With my current approach, that made me uncomfortable
using pg_amop, since I'd need to invent a new amoppurpose. Intersect isn't used for search or
ordering, just for foreign keys (and later for temporal update/delete). Similarly, This new enum is
COMPARE_*, and intersect isn't a comparison.
Is there anything we can do about that? Is pg_amop really only for indexes, or can I add a new
amoppurpose and use it? And does this enum need to be only for indexes, or can it be for other
things too? Maybe instead of COMPARE_* it can be OPERATION_*.
Today I just hardcode the built-in intersect operator for ranges and multiranges, but I would rather
have a solution that lets people define foreign keys on arbitrary types. Temporal primary keys
support arbitrary types (well other than the empty-range issue, which is something I think we can
resolve), as do temporal UPDATE and DELETE, so getting foreign keys to support them would complete
the picture. To me this goal is more faithful to Postgres's heritage as an "object-relational"
database, a philosophy that has made it so extensible.
As I mentioned, UPDATE/DELETE FOR PORTION OF also needs intersect. But it only needs a proc, not an
operator. For that I added another GiST support proc, intersect, that is just the same proc that
implements the operator. But if foreign keys had a way to get an operator for any type, then FOR
PORTION OF could use that too, because an operator implies a proc (but not vice versa).
Then I'd need one less support function.
Or here is another approach: I was thinking that instead of a `stratnum` support proc, used to get
strategy numbers (whether by well-known stratnum or by COMPARE_*), we could have a support proc that
*returns operator oids* (again, whether by well-known strategy number or by COMPARE_*). Instead of
`stratnum` we could call it something like `get_operator`. I can't think of any other use for
strategy numbers than looking up operators. The existing callsites of GistTranslateStratnum all
immediately call get_opfamily_member (and Peter's patches don't change that). What if we had a
higher-level wrapper of get_opfamily_member that took the same parameters, but used this new support
function (if defined) when called for non-btree AMs?
Then (1) foreign keys could use it get the intersect operator (2) I could drop the FOR PORTION OF
support proc. (The stratnum support proc only exists in v18devel, so it's still easy to change.)
One problem is: how do extension authors know the oids of their operators? I guess they could look
them up in pg_operator (via pg_depend?) But we should acknowledge that it's harder for them to get
oids than for core. Or maybe this isn't such a big deal after all: pg_operator is unique on
(oprname, oprleft, oprright, oprnamespace).
Are there other problems? It does seem like a risk to have a second "source of truth", especially
one that is implemented as code rather than data. What if the code changes? But still I like this
approach. Non-btree indexes can't really use pg_amop anyway, because amopstrategy doesn't mean
anything. I'm happy to make a commit replacing the stratnum support proc with get_operator as I've
described it.
But then what is its input? If it's a stratnum, there is no problem: I'll add an
RTIntersectStrategyNumber. If it's a COMPARE_*, then we still want a non-index-specific name for
this enum.
Either approach to getting an intersect operator seems fine to me: using pg_amop with a third
amoppurpose, or replacing stratnum with get_operator. But both seem to expand an index-specific
abstraction to include something new. What do you think?
# Can you create unique GiST indexes or not?
Postgres lets you ask about the capabilities of different AMs. For instance:
postgres=# select amname, pg_indexam_has_property(oid, 'can_unique') from pg_am where amtype = 'i';
amname | pg_indexam_has_property
--------+-------------------------
btree | t
hash | f
gist | f
gin | f
spgist | f
brin | f
(6 rows)
So if GiST can't support unique indexes, why can you do this?:
postgres=# create extension btree_gist;
CREATE EXTENSION
postgres=# create table t (id int, valid_at daterange, unique (id, valid_at without overlaps));
CREATE TABLE
postgres=# \d t
Table "public.t"
Column | Type | Collation | Nullable | Default
----------+-----------+-----------+----------+---------
id | integer | | |
valid_at | daterange | | |
Indexes:
"t_id_valid_at_key" UNIQUE (id, valid_at WITHOUT OVERLAPS)
And also:
postgres=# select indisunique from pg_index where indexrelid = 't_id_valid_at_key'::regclass;
indisunique
-------------
t
But:
postgres=# create unique index idx_t on t using gist (id, valid_at);
ERROR: access method "gist" does not support unique indexes
It seems like we have an inconsistency, as Matthew van de Meent brought up here (cc'd):
https://www.postgresql.org/message-id/CAEze2WiD%2BU1BuJDLGL%3DFXxa8hDxNALVE6Jij0cNXjp10Q%3DnZHw%40ma...
The reason is that GiST *might* support unique indexes, but it depends on the opclass. (And note
GiST has supported effectively-unique indexes via exclusion constraints for a long time.)
From another perspective, the error message is correct: you can't create a unique GiST index using
CREATE UNIQUE INDEX. You can only create a UNIQUE/PRIMARY KEY *constraint*, which creates a unique
GiST index to implement it. But that is not a very satisfying answer.
But since we are improving stratnums, we could allow the above command, even with what is already
committed today: just see if we can find an equals operator, and create the index. I'm interested in
working on that myself, but the other temporal patches are a higher priority.
In the meantime perhaps we can improve the error message to sound less contradictory. How about
"access method 'gist' does not support unique indexes without a WITHOUT OVERLAPS constraint"?
And anyway, creating simply-unique GiST indexes is not very valuable, when you could just create a
btree index instead. The main reason is legalistic, so we aren't guilty of contradicting our error
message. It doesn't satisfy Matthew's use-case. He wants to avoid long exclusive locks by doing
CREATE INDEX CONCURRENTLY then ADD CONSTRAINT USING INDEX. But then you'd need to CREATE UNIQUE
INDEX with overlaps for the last element, not equals. There is no syntax for that. Perhaps we could
borrow the syntax for constraints: CREATE UNIQUE INDEX idx ON t USING gist (id, valid_at WITHOUT
OVERLAPS). It's not part of the standard though, and it might have parse conflicts with COLLATE or
an opclass name.
But that *still* doesn't get there, because (1) the index has to enforce (temporal) uniqueness
before you add the constraint (2) it needs to allow CONCURRENTLY. Exclusion constraints don't
enforce their checks while updating the index, but with a separate search (IIRC), and the details
would probably have to move from pg_constraint.conexclop to pg_index.indexclop (which doesn't exist
today). To pre-create *any* exclusion constraint index, we'd need a more general syntax than WITHOUT
OVERLAPS. Something like (id WITH =, valid_at WITH &&).
And then that has to work with CONCURRENTLY. You *can* create a GiST index CONCURRENTLY today, but
not one that enforces anything. There was work already to allow CONCURRENTLY to REINDEX exclusion
constraint indexes, but I assume it's tricky, because it has stalled twice:
see
https://www.postgresql.org/message-id/flat/CAB7nPqS%2BWYN021oQHd9GPe_5dSVcVXMvEBW_E2AV9OOEwggMHw%40m...
from 2012 and
https://www.postgresql.org/message-id/flat/60052986-956b-4478-45ed-8bd119e9b9cf%402ndquadrant.com#74...
from 2019. So there are some substantial gaps to fill, and I think the first step for now is just
updating the error message.
What else can we do here? We have pg_indexam_has_property, pg_index_has_property, and
pg_index_column_has_property. Do we want a pg_opclass_has_property?
Or here is a more modest suggestion: we could permit pg_index_has_property to answer can_unique
questions. Today it returns NULL for that property. Perhaps it should (by default) answer whatever
the AM can do. Then we could update gistproperty (which today only answers column-level inquiries),
and return indisunique. To be honest that feels like an oversight that should have been included in
the temporal primary key patch. I'll sign up to do that if people think it makes sense.
Another thought: should pg_indexam_has_property(783, 'can_unique') return NULL, on the theory that
NULL means "unknown", and as of pg 18 GiST uniqueness is no longer "false" but now "it depends"?
Maybe that is too cute. If we did that, probably it should be paired with a way to get a definitive
answer for the scenario you care about (without creating an index first). pg_opclass_has_property
could do that. Or maybe we add a second pg_indexam_has_property that also takes an array of opclass
oids?
# Logical replication
I had to teach logical replication how to handle temporal index constraints. It wants a unique
identifier for REPLICA IDENTITY. Even with WITHOUT OVERLAPS, the combination of index keys do
identify a unique record. But again, logical replication needs to know which operators implement
equals for non-btree indexes. I think the discussion in this thread has already covered this
sufficiently. Peter's COMPARE_* change doesn't introduce any problems from what I can see.
# Updating opclasses
Today you can alter an opfamily, but you can't alter an opclass. So to add stratnum support
functions, the btree_gist upgrade script does this:
ALTER OPERATOR FAMILY gist_int4_ops USING gist ADD
FUNCTION 12 (int4, int4) gist_stratnum_btree (int2) ;
Past upgrade scripts have done similar things.
But that doesn't achieve quite the same thing as putting the function in CREATE OPERATOR CLASS. Do
we want a way to attach a new support function to an *opclass*? There are some problems to solve
with staleness, the relcache for example I think, but nothing seems insurmountable. Maybe it risks
incoherent indexes, if you change how they are computed partway through, but if so it was probably
true already with ALTER OPERATOR FAMILY.
# Empty ranges
This is probably not relevant to this discussion, but just in case: in the v17 cycle we had to
revert temporal PKs because empty ranges allowed duplicate records (because 'empty' && 'empty' is
false, so there was no conflict). The solution was to reject empty ranges in a temporal PK or UNIQUE
constraint, where they really don't make sense. Is there something generalized the index AM API
could offer for this kind of thing?
Yours,
--
Paul ~{:-)
[email protected]
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Index AM API cleanup
@ 2025-01-15 14:31 Peter Eisentraut <[email protected]>
parent: Peter Eisentraut <[email protected]>
1 sibling, 1 reply; 14+ messages in thread
From: Peter Eisentraut @ 2025-01-15 14:31 UTC (permalink / raw)
To: Mark Dilger <[email protected]>; +Cc: pgsql-hackers; Kirill Reshke <[email protected]>; Andrew Dunstan <[email protected]>; Alex Wang <[email protected]>; Paul A Jungwirth <[email protected]>
On 04.12.24 15:49, Peter Eisentraut wrote:
> On 27.11.24 13:57, Peter Eisentraut wrote:
>> I think, however, that we should rename RowCompareType. Otherwise,
>> it's just going to be confusing forevermore. I suggest to rename it
>> simply to CompareType.
>
>> I'm going to try to code up the gist support on top of this patch set
>> to make sure that it will fit well. I'll report back.
>
> Here is a patch set in that direction. It renames RowCompareType to
> CompareType and updates the surrounding commentary a bit. And then I'm
> changing the gist strategy mapping to use the CompareType values instead
> of the RT* strategy numbers. Seeing this now, I like this a lot better
> than what we have now, because it makes it clearer in the API and the
> code what is a real strategy number and what's a different kind of
> thing. (This isn't entirely the above-mentioned integration of the gist
> support into your patch set yet, but it's a meaningful part of it.)
I have committed these, and I'll continue working my way through this
patch set now.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Index AM API cleanup
@ 2025-01-15 21:15 Nathan Bossart <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Nathan Bossart @ 2025-01-15 21:15 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Mark Dilger <[email protected]>; pgsql-hackers; Kirill Reshke <[email protected]>; Andrew Dunstan <[email protected]>; Alex Wang <[email protected]>; Paul A Jungwirth <[email protected]>
On Wed, Jan 15, 2025 at 03:31:12PM +0100, Peter Eisentraut wrote:
> On 04.12.24 15:49, Peter Eisentraut wrote:
>> Here is a patch set in that direction. It renames RowCompareType to
>> CompareType and updates the surrounding commentary a bit. And then I'm
>> changing the gist strategy mapping to use the CompareType values instead
>> of the RT* strategy numbers. Seeing this now, I like this a lot better
>> than what we have now, because it makes it clearer in the API and the
>> code what is a real strategy number and what's a different kind of
>> thing. (This isn't entirely the above-mentioned integration of the gist
>> support into your patch set yet, but it's a meaningful part of it.)
>
> I have committed these, and I'll continue working my way through this patch
> set now.
cfbot's cpluspluscheck step seems to dislike the new CompareType enum
declaraction [0]. IIUC that isn't valid in C++.
[0] https://api.cirrus-ci.com/v1/task/4946960681009152/logs/headers_cpluspluscheck.log
--
nathan
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Index AM API cleanup
@ 2025-01-15 22:14 Peter Eisentraut <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Peter Eisentraut @ 2025-01-15 22:14 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Mark Dilger <[email protected]>; pgsql-hackers; Kirill Reshke <[email protected]>; Andrew Dunstan <[email protected]>; Alex Wang <[email protected]>; Paul A Jungwirth <[email protected]>
On 15.01.25 22:15, Nathan Bossart wrote:
> On Wed, Jan 15, 2025 at 03:31:12PM +0100, Peter Eisentraut wrote:
>> On 04.12.24 15:49, Peter Eisentraut wrote:
>>> Here is a patch set in that direction. It renames RowCompareType to
>>> CompareType and updates the surrounding commentary a bit. And then I'm
>>> changing the gist strategy mapping to use the CompareType values instead
>>> of the RT* strategy numbers. Seeing this now, I like this a lot better
>>> than what we have now, because it makes it clearer in the API and the
>>> code what is a real strategy number and what's a different kind of
>>> thing. (This isn't entirely the above-mentioned integration of the gist
>>> support into your patch set yet, but it's a meaningful part of it.)
>>
>> I have committed these, and I'll continue working my way through this patch
>> set now.
>
> cfbot's cpluspluscheck step seems to dislike the new CompareType enum
> declaraction [0]. IIUC that isn't valid in C++.
Fixed, thanks.
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2025-01-15 22:14 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-04-03 05:10 [PATCH 03/23] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2022-04-03 05:10 [PATCH 13/21] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2022-04-03 05:10 [PATCH 05/11] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2022-04-03 05:10 [PATCH 03/23] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2022-04-03 05:10 [PATCH 03/25] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2022-04-03 05:10 [PATCH 13/21] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2022-04-03 05:10 [PATCH 7/8] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2024-11-27 12:57 Re: Index AM API cleanup Peter Eisentraut <[email protected]>
2024-11-27 15:51 ` Re: Index AM API cleanup Mark Dilger <[email protected]>
2024-12-04 14:49 ` Re: Index AM API cleanup Peter Eisentraut <[email protected]>
2025-01-05 23:20 ` Re: Index AM API cleanup Paul Jungwirth <[email protected]>
2025-01-15 14:31 ` Re: Index AM API cleanup Peter Eisentraut <[email protected]>
2025-01-15 21:15 ` Re: Index AM API cleanup Nathan Bossart <[email protected]>
2025-01-15 22:14 ` Re: Index AM API cleanup Peter Eisentraut <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox