public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v1 01/12] jit: Instrument function purpose separately, add tracking of modules. 13+ messages / 3 participants [nested] [flat]
* [PATCH v1 01/12] jit: Instrument function purpose separately, add tracking of modules. @ 2019-09-26 18:44 Andres Freund <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Andres Freund @ 2019-09-26 18:44 UTC (permalink / raw) Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/commands/explain.c | 24 +++++++++++++++++++++++- src/backend/jit/jit.c | 3 +++ src/backend/jit/llvm/llvmjit.c | 2 ++ src/backend/jit/llvm/llvmjit_deform.c | 1 + src/backend/jit/llvm/llvmjit_expr.c | 1 + src/include/jit/jit.h | 11 ++++++++++- 6 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 62fb3434a32..ef65035bfba 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -825,7 +825,26 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, appendStringInfoString(es->str, "JIT:\n"); es->indent += 1; - ExplainPropertyInteger("Functions", NULL, ji->created_functions, es); + /* having to emit code more than once has performance consequences */ + if (ji->created_modules > 1) + ExplainPropertyInteger("Modules", NULL, ji->created_modules, es); + + appendStringInfoSpaces(es->str, es->indent * 2); + appendStringInfo(es->str, "Functions: %zu", ji->created_functions); + if (ji->created_expr_functions > 0 || ji->created_deform_functions) + { + appendStringInfoString(es->str, " ("); + if (ji->created_expr_functions) + { + appendStringInfo(es->str, "%zu for expression evaluation", ji->created_expr_functions); + if (ji->created_deform_functions) + appendStringInfoString(es->str, ", "); + } + if (ji->created_deform_functions) + appendStringInfo(es->str, "%zu for tuple deforming", ji->created_deform_functions); + appendStringInfoChar(es->str, ')'); + } + appendStringInfoChar(es->str, '\n'); appendStringInfoSpaces(es->str, es->indent * 2); appendStringInfo(es->str, "Options: %s %s, %s %s, %s %s, %s %s\n", @@ -851,7 +870,10 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, else { ExplainPropertyInteger("Worker Number", NULL, worker_num, es); + ExplainPropertyInteger("Modules", NULL, ji->created_modules, es); ExplainPropertyInteger("Functions", NULL, ji->created_functions, es); + ExplainPropertyInteger("Expression Functions", NULL, ji->created_expr_functions, es); + ExplainPropertyInteger("Deforming Functions", NULL, ji->created_deform_functions, es); ExplainOpenGroup("Options", "Options", true, es); ExplainPropertyBool("Inlining", jit_flags & PGJIT_INLINE, es); diff --git a/src/backend/jit/jit.c b/src/backend/jit/jit.c index 43e65b1a543..63c709002d8 100644 --- a/src/backend/jit/jit.c +++ b/src/backend/jit/jit.c @@ -186,7 +186,10 @@ jit_compile_expr(struct ExprState *state) void InstrJitAgg(JitInstrumentation *dst, JitInstrumentation *add) { + dst->created_modules += add->created_modules; dst->created_functions += add->created_functions; + dst->created_expr_functions += add->created_expr_functions; + dst->created_deform_functions += add->created_deform_functions; INSTR_TIME_ADD(dst->generation_counter, add->generation_counter); INSTR_TIME_ADD(dst->inlining_counter, add->inlining_counter); INSTR_TIME_ADD(dst->optimization_counter, add->optimization_counter); diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 82c4afb7011..5489e118041 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -212,6 +212,8 @@ llvm_mutable_module(LLVMJitContext *context) context->module = LLVMModuleCreateWithName("pg"); LLVMSetTarget(context->module, llvm_triple); LLVMSetDataLayout(context->module, llvm_layout); + + context->base.instr.created_modules++; } return context->module; diff --git a/src/backend/jit/llvm/llvmjit_deform.c b/src/backend/jit/llvm/llvmjit_deform.c index 835aea83e97..80a85858524 100644 --- a/src/backend/jit/llvm/llvmjit_deform.c +++ b/src/backend/jit/llvm/llvmjit_deform.c @@ -101,6 +101,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, mod = llvm_mutable_module(context); funcname = llvm_expand_funcname(context, "deform"); + context->base.instr.created_deform_functions++; /* * Check which columns have to exist, so we don't have to check the row's diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index 30133634c70..7efc8f23ee3 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -144,6 +144,7 @@ llvm_compile_expr(ExprState *state) b = LLVMCreateBuilder(); funcname = llvm_expand_funcname(context, "evalexpr"); + context->base.instr.created_expr_functions++; /* Create the signature and function */ { diff --git a/src/include/jit/jit.h b/src/include/jit/jit.h index d879cef20f3..668f965cb0a 100644 --- a/src/include/jit/jit.h +++ b/src/include/jit/jit.h @@ -26,9 +26,18 @@ typedef struct JitInstrumentation { - /* number of emitted functions */ + /* number of modules (i.e. separate optimize / link cycles) created */ + size_t created_modules; + + /* number of functions generated */ size_t created_functions; + /* number of expression evaluation functions generated */ + size_t created_expr_functions; + + /* number of tuple deforming functions generated */ + size_t created_deform_functions; + /* accumulated time to generate code */ instr_time generation_counter; -- 2.23.0.162.gf1d4a28250 --kgppz6muakthis74 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v1-0002-Refactor-explain.c-to-pass-ExprState-down-to-show.patch" ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH v2 1/8] jit: Instrument function purpose separately, add tracking of modules. @ 2019-09-26 18:44 Andres Freund <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Andres Freund @ 2019-09-26 18:44 UTC (permalink / raw) Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/commands/explain.c | 24 +++++++++++++++++++++++- src/backend/jit/jit.c | 3 +++ src/backend/jit/llvm/llvmjit.c | 2 ++ src/backend/jit/llvm/llvmjit_deform.c | 1 + src/backend/jit/llvm/llvmjit_expr.c | 1 + src/include/jit/jit.h | 11 ++++++++++- 6 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 62fb3434a32..ef65035bfba 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -825,7 +825,26 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, appendStringInfoString(es->str, "JIT:\n"); es->indent += 1; - ExplainPropertyInteger("Functions", NULL, ji->created_functions, es); + /* having to emit code more than once has performance consequences */ + if (ji->created_modules > 1) + ExplainPropertyInteger("Modules", NULL, ji->created_modules, es); + + appendStringInfoSpaces(es->str, es->indent * 2); + appendStringInfo(es->str, "Functions: %zu", ji->created_functions); + if (ji->created_expr_functions > 0 || ji->created_deform_functions) + { + appendStringInfoString(es->str, " ("); + if (ji->created_expr_functions) + { + appendStringInfo(es->str, "%zu for expression evaluation", ji->created_expr_functions); + if (ji->created_deform_functions) + appendStringInfoString(es->str, ", "); + } + if (ji->created_deform_functions) + appendStringInfo(es->str, "%zu for tuple deforming", ji->created_deform_functions); + appendStringInfoChar(es->str, ')'); + } + appendStringInfoChar(es->str, '\n'); appendStringInfoSpaces(es->str, es->indent * 2); appendStringInfo(es->str, "Options: %s %s, %s %s, %s %s, %s %s\n", @@ -851,7 +870,10 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, else { ExplainPropertyInteger("Worker Number", NULL, worker_num, es); + ExplainPropertyInteger("Modules", NULL, ji->created_modules, es); ExplainPropertyInteger("Functions", NULL, ji->created_functions, es); + ExplainPropertyInteger("Expression Functions", NULL, ji->created_expr_functions, es); + ExplainPropertyInteger("Deforming Functions", NULL, ji->created_deform_functions, es); ExplainOpenGroup("Options", "Options", true, es); ExplainPropertyBool("Inlining", jit_flags & PGJIT_INLINE, es); diff --git a/src/backend/jit/jit.c b/src/backend/jit/jit.c index 43e65b1a543..63c709002d8 100644 --- a/src/backend/jit/jit.c +++ b/src/backend/jit/jit.c @@ -186,7 +186,10 @@ jit_compile_expr(struct ExprState *state) void InstrJitAgg(JitInstrumentation *dst, JitInstrumentation *add) { + dst->created_modules += add->created_modules; dst->created_functions += add->created_functions; + dst->created_expr_functions += add->created_expr_functions; + dst->created_deform_functions += add->created_deform_functions; INSTR_TIME_ADD(dst->generation_counter, add->generation_counter); INSTR_TIME_ADD(dst->inlining_counter, add->inlining_counter); INSTR_TIME_ADD(dst->optimization_counter, add->optimization_counter); diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 82c4afb7011..5489e118041 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -212,6 +212,8 @@ llvm_mutable_module(LLVMJitContext *context) context->module = LLVMModuleCreateWithName("pg"); LLVMSetTarget(context->module, llvm_triple); LLVMSetDataLayout(context->module, llvm_layout); + + context->base.instr.created_modules++; } return context->module; diff --git a/src/backend/jit/llvm/llvmjit_deform.c b/src/backend/jit/llvm/llvmjit_deform.c index 835aea83e97..80a85858524 100644 --- a/src/backend/jit/llvm/llvmjit_deform.c +++ b/src/backend/jit/llvm/llvmjit_deform.c @@ -101,6 +101,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, mod = llvm_mutable_module(context); funcname = llvm_expand_funcname(context, "deform"); + context->base.instr.created_deform_functions++; /* * Check which columns have to exist, so we don't have to check the row's diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index d09324637b9..4ba8c78cbc9 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -144,6 +144,7 @@ llvm_compile_expr(ExprState *state) b = LLVMCreateBuilder(); funcname = llvm_expand_funcname(context, "evalexpr"); + context->base.instr.created_expr_functions++; /* Create the signature and function */ { diff --git a/src/include/jit/jit.h b/src/include/jit/jit.h index d879cef20f3..668f965cb0a 100644 --- a/src/include/jit/jit.h +++ b/src/include/jit/jit.h @@ -26,9 +26,18 @@ typedef struct JitInstrumentation { - /* number of emitted functions */ + /* number of modules (i.e. separate optimize / link cycles) created */ + size_t created_modules; + + /* number of functions generated */ size_t created_functions; + /* number of expression evaluation functions generated */ + size_t created_expr_functions; + + /* number of tuple deforming functions generated */ + size_t created_deform_functions; + /* accumulated time to generate code */ instr_time generation_counter; -- 2.23.0.385.gbc12974a89 --ga6shgqrocqphdjc Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0002-Refactor-explain.c-to-pass-ExprState-down-to-show.patch" ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Incorrect estimation of HashJoin rows resulted from inaccurate small table statistics @ 2023-06-17 13:45 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Tomas Vondra @ 2023-06-17 13:45 UTC (permalink / raw) To: Quan Zongliang <[email protected]>; Tom Lane <[email protected]>; +Cc: pgsql-hackers On 6/17/23 02:02, Quan Zongliang wrote: > > > On 2023/6/17 06:46, Tom Lane wrote: >> Quan Zongliang <[email protected]> writes: >>> Perhaps we should discard this (dups cnt > 1) restriction? >> >> That's not going to happen on the basis of one test case that you >> haven't even shown us. The implications of doing it are very unclear. >> In particular, I seem to recall that there are bits of logic that >> depend on the assumption that MCV entries always represent more than >> one row. The nmultiple calculation Tomas referred to may be failing >> because of that, but I'm worried about there being other places. >> I don't recall any logic that'd outright fail with MCVs containing single-row groups, and I haven't noticed anything obvious in analyze.c during a cursory search. Maybe the paper analyze_mcv_list builds on makes some assumptions? Not sure. However, compute_distinct_stats() doesn't seem to have such protection against single-row MCV groups, so if that's wrong we kinda already have the issue I think (admittedly, compute_distinct_stats is much less used than compute_scalar_stats). > > The statistics for the other table look like this: > stadistinct | 6 > stanumbers1 | {0.50096667,0.49736667,0.0012} > stavalues1 | {v22,v23,v5} > > The value that appears twice in the small table (v1 and v2) does not > appear here. The stadistinct's true value is 18 instead of 6 (three > values in the small table do not appear here). > > When calculating the selectivity: > if (nd2 > sslot2->nvalues) > totalsel1 += unmatchfreq1 * otherfreq2 / (nd2 - sslot2->nvalues); > > totalsel1 = 0 > nd2 = 21 > sslot2->nvalues = 2 > unmatchfreq1 = 0.99990002016420476 > otherfreq2 = 0.82608695328235626 > > result: totalsel1 = 0.043473913749706022 > rows = 0.043473913749706022 * 23 * 2,000,000 = 1999800 > Attached is a script reproducing this. I think the fundamental issue here is that the most common element of the large table - v22 (~50%) is not in the tiny one at all. IIRC the join estimation assumes the domain of one table is a subset of the other. The values 22 / 23 violate that assumption, unfortunately. Including all values into the small MCV fix this because then otherfreq1 = 0.0 and that simply eliminates the impact of stuff that didn't have a match between the two MCV lists. Which mitigates the violated assumption. But once the small table gets too large for the MCV, this won't work that well - it probably helps a bit, as it makes otherfreq1 smaller. Which doesn't mean it's useless, but it's likely a rare combination that a table is (and remains) smaller than MCV, and the large table contains values without a match in the smaller one (think foreign keys). > >> Basically, you're proposing a rather fundamental change in the rules >> by which Postgres has gathered statistics for decades. You need to >> bring some pretty substantial evidence to support that. The burden >> of proof is on you, not on the status quo. >> Right. It's a good example of a "quick hack" fixing one particular case, without considering the consequences on other cases too much. Good as a starting point, but plenty of legwork to do. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company Attachments: [application/sql] script.sql (564B, ../../[email protected]/2-script.sql) download ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH 2/7] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v42-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH v45 2/8] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --cldir6umqisr673d Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v45-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH v44b 2/4] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --fot5plqmb33ey33c Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v44b-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH v44 02/10] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --gwom7bl7ogtszo4k Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v44-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH v44 02/10] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --gwom7bl7ogtszo4k Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v44-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH v44b 2/4] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --fot5plqmb33ey33c Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v44b-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH 2/7] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v42-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH v43 2/7] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --nzintiedl6o4kcyp Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v43-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH v45 2/8] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --cldir6umqisr673d Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v45-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH v43 2/7] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --nzintiedl6o4kcyp Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v43-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2026-03-11 13:53 UTC | newest] Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-09-26 18:44 [PATCH v1 01/12] jit: Instrument function purpose separately, add tracking of modules. Andres Freund <[email protected]> 2019-09-26 18:44 [PATCH v2 1/8] jit: Instrument function purpose separately, add tracking of modules. Andres Freund <[email protected]> 2023-06-17 13:45 Re: Incorrect estimation of HashJoin rows resulted from inaccurate small table statistics Tomas Vondra <[email protected]> 2026-03-11 13:53 [PATCH 2/7] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v45 2/8] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v44b 2/4] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v44 02/10] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v44 02/10] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v44b 2/4] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH 2/7] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v43 2/7] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v45 2/8] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v43 2/7] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[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