public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 21/23] cirrus/warnings: use ./configure cache in headerscheck.. 5+ messages / 3 participants [nested] [flat]
* [PATCH 21/23] cirrus/warnings: use ./configure cache in headerscheck.. @ 2022-07-19 17:38 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Justin Pryzby @ 2022-07-19 17:38 UTC (permalink / raw) This is desirable since since configure is slow. It's necessary for the environment variables to match, so disable ccache a different way. ci-os-only: warnings --- .cirrus.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 4f0695e455d..6f743ee03c0 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -633,11 +633,13 @@ task: ### always: headers_headerscheck_script: | + export CCACHE_DISABLE=1 time ./configure \ + --cache gcc.cache \ ${LINUX_CONFIGURE_FEATURES} \ --without-icu \ --quiet \ - CC="gcc" CXX"=g++" CLANG="clang" + CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang" make -s -j${BUILD_JOBS} clean time make -s headerscheck EXTRAFLAGS='-fmax-errors=10' headers_cpluspluscheck_script: | -- 2.17.1 --tKkaNMvYmhQvRCRK Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0022-cirrus-warnings-move-use-a-single-common-always-bloc.patch" ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH 24/25] cirrus/warnings: use ./configure cache in headerscheck.. @ 2022-07-19 17:38 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Justin Pryzby @ 2022-07-19 17:38 UTC (permalink / raw) This is desirable since since configure is slow. It's necessary for the environment variables to match, so disable ccache a different way. ci-os-only: warnings --- .cirrus.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 0421d56ca70..81822c58bec 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -627,11 +627,13 @@ task: ### always: headers_headerscheck_script: | + export CCACHE_DISABLE=1 time ./configure \ + --cache gcc.cache \ ${LINUX_CONFIGURE_FEATURES} \ --without-icu \ --quiet \ - CC="gcc" CXX"=g++" CLANG="clang" + CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang" make -s -j${BUILD_JOBS} clean time make -s headerscheck EXTRAFLAGS='-fmax-errors=10' headers_cpluspluscheck_script: | -- 2.17.1 --IS0zKkzwUGydFO0o Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0025-cirrus-warnings-move-use-a-single-common-always-bloc.patch" ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH 21/23] cirrus/warnings: use ./configure cache in headerscheck.. @ 2022-07-19 17:38 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Justin Pryzby @ 2022-07-19 17:38 UTC (permalink / raw) This is desirable since since configure is slow. It's necessary for the environment variables to match, so disable ccache a different way. ci-os-only: warnings --- .cirrus.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 4f0695e455d..6f743ee03c0 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -633,11 +633,13 @@ task: ### always: headers_headerscheck_script: | + export CCACHE_DISABLE=1 time ./configure \ + --cache gcc.cache \ ${LINUX_CONFIGURE_FEATURES} \ --without-icu \ --quiet \ - CC="gcc" CXX"=g++" CLANG="clang" + CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang" make -s -j${BUILD_JOBS} clean time make -s headerscheck EXTRAFLAGS='-fmax-errors=10' headers_cpluspluscheck_script: | -- 2.17.1 --tKkaNMvYmhQvRCRK Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0022-cirrus-warnings-move-use-a-single-common-always-bloc.patch" ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Removing unneeded self joins @ 2024-02-22 08:51 Andrei Lepikhov <[email protected]> 0 siblings, 1 reply; 5+ messages in thread From: Andrei Lepikhov @ 2024-02-22 08:51 UTC (permalink / raw) To: Richard Guo <[email protected]>; +Cc: Alexander Korotkov <[email protected]>; Alexander Lakhin <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Michał Kłeczek <[email protected]>; PostgreSQL Hackers <[email protected]>; Tom Lane <[email protected]> On 21/2/2024 14:26, Richard Guo wrote: > I think the right fix for these issues is to introduce a new element > 'sublevels_up' in ReplaceVarnoContext, and enhance replace_varno_walker > to 1) recurse into subselects with sublevels_up increased, and 2) > perform the replacement only when varlevelsup is equal to sublevels_up. This code looks good. No idea how we have lost it before. > > While writing the fix, I noticed some outdated comments. Such as in > remove_rel_from_query, the first for loop updates otherrel's attr_needed > as well as lateral_vars, but the comment only mentions attr_needed. So > this patch also fixes some outdated comments. Thanks, looks good. > > While writing the test cases, I found that the test cases for SJE are > quite messy. Below are what I have noticed: > > * There are several test cases using catalog tables like pg_class, > pg_stats, pg_index, etc. for testing join removal. I don't see a reason > why we need to use catalog tables, and I think this just raises the risk > of instability. I see only one unusual query with the pg_class involved. > > * In many test cases, a mix of uppercase and lowercase keywords is used > in one query. I think it'd better to maintain consistency by using > either all uppercase or all lowercase keywords in one query. I see uppercase -> lowercase change: select t1.*, t2.a as ax from sj t1 join sj t2 and lowercase -> uppercase in many other cases: explain (costs off) I guess it is a matter of taste, so give up for the committer decision. Technically, it's OK. > > * In most situations, we verify the plan and the output of a query like: > > explain (costs off) > select ...; > select ...; > > The two select queries are supposed to be the same. But in the SJE test > cases, I have noticed instances where the two select queries differ from > each other. > > This patch also includes some cosmetic tweaks for SJE test cases. It > does not change the test cases using catalog tables though. I think > that should be a seperate patch. I can't assess the necessity of changing these dozens of lines of code because I follow another commenting style, but technically, it's still OK. -- regards, Andrei Lepikhov Postgres Professional ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Removing unneeded self joins @ 2024-02-23 22:36 Alexander Korotkov <[email protected]> parent: Andrei Lepikhov <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Alexander Korotkov @ 2024-02-23 22:36 UTC (permalink / raw) To: Andrei Lepikhov <[email protected]>; +Cc: Richard Guo <[email protected]>; Alexander Lakhin <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Michał Kłeczek <[email protected]>; PostgreSQL Hackers <[email protected]>; Tom Lane <[email protected]> On Thu, Feb 22, 2024 at 10:51 AM Andrei Lepikhov <[email protected]> wrote: > On 21/2/2024 14:26, Richard Guo wrote: > > I think the right fix for these issues is to introduce a new element > > 'sublevels_up' in ReplaceVarnoContext, and enhance replace_varno_walker > > to 1) recurse into subselects with sublevels_up increased, and 2) > > perform the replacement only when varlevelsup is equal to sublevels_up. > This code looks good. No idea how we have lost it before. Thanks to Richard for the patch and to Andrei for review. I also find code looking good. Pushed with minor edits from me. ------ Regards, Alexander Korotkov ^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2024-02-23 22:36 UTC | newest] Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-07-19 17:38 [PATCH 21/23] cirrus/warnings: use ./configure cache in headerscheck.. Justin Pryzby <[email protected]> 2022-07-19 17:38 [PATCH 24/25] cirrus/warnings: use ./configure cache in headerscheck.. Justin Pryzby <[email protected]> 2022-07-19 17:38 [PATCH 21/23] cirrus/warnings: use ./configure cache in headerscheck.. Justin Pryzby <[email protected]> 2024-02-22 08:51 Re: Removing unneeded self joins Andrei Lepikhov <[email protected]> 2024-02-23 22:36 ` Re: Removing unneeded self joins Alexander Korotkov <[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