public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v24 2/7] psql: Add HIDE_COMPRESSAM for regress testing
5+ messages / 3 participants
[nested] [flat]
* [PATCH v24 2/7] psql: Add HIDE_COMPRESSAM for regress testing
@ 2021-02-14 02:48 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Justin Pryzby @ 2021-02-14 02:48 UTC (permalink / raw)
This avoids churn in regression output, and allows testing with alternate
compression AMs.
---
doc/src/sgml/ref/psql-ref.sgml | 11 +++++++++++
src/bin/psql/describe.c | 1 +
src/bin/psql/help.c | 2 ++
src/bin/psql/settings.h | 1 +
src/bin/psql/startup.c | 9 +++++++++
src/test/regress/pg_regress_main.c | 4 ++--
6 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 13c1edfa4d..66dcb1b33b 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3863,6 +3863,17 @@ bar
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><varname>HIDE_COMPRESSAM</varname></term>
+ <listitem>
+ <para>
+ If this variable is set to <literal>true</literal>, column's
+ compression access method details are not displayed. This is mainly
+ useful for regression tests.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><varname>HIDE_TABLEAM</varname></term>
<listitem>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index ba464d463e..b835b0cf76 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1897,6 +1897,7 @@ describeOneTableDetails(const char *schemaname,
/* compresssion info */
if (pset.sversion >= 140000 &&
+ !pset.hide_compressam &&
(tableinfo.relkind == RELKIND_RELATION ||
tableinfo.relkind == RELKIND_PARTITIONED_TABLE ||
tableinfo.relkind == RELKIND_MATVIEW))
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index e44120bf76..a818ee5503 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -375,6 +375,8 @@ helpVariables(unsigned short int pager)
" true if last query failed, else false\n"));
fprintf(output, _(" FETCH_COUNT\n"
" the number of result rows to fetch and display at a time (0 = unlimited)\n"));
+ fprintf(output, _(" HIDE_COMPRESSAM\n"
+ " if set, compression access methods are not displayed\n"));
fprintf(output, _(" HIDE_TABLEAM\n"
" if set, table access methods are not displayed\n"));
fprintf(output, _(" HISTCONTROL\n"
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index d65990059d..9755e8eac6 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -134,6 +134,7 @@ typedef struct _psqlSettings
bool quiet;
bool singleline;
bool singlestep;
+ bool hide_compressam;
bool hide_tableam;
int fetch_count;
int histsize;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 780479c8d7..554b64367d 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -1159,6 +1159,12 @@ show_context_hook(const char *newval)
return true;
}
+static bool
+hide_compressam_hook(const char *newval)
+{
+ return ParseVariableBool(newval, "HIDE_COMPRESSAM", &pset.hide_compressam);
+}
+
static bool
hide_tableam_hook(const char *newval)
{
@@ -1227,6 +1233,9 @@ EstablishVariableSpace(void)
SetVariableHooks(pset.vars, "SHOW_CONTEXT",
show_context_substitute_hook,
show_context_hook);
+ SetVariableHooks(pset.vars, "HIDE_COMPRESSAM",
+ bool_substitute_hook,
+ hide_compressam_hook);
SetVariableHooks(pset.vars, "HIDE_TABLEAM",
bool_substitute_hook,
hide_tableam_hook);
diff --git a/src/test/regress/pg_regress_main.c b/src/test/regress/pg_regress_main.c
index 8dc4941c24..07fd3f6a4d 100644
--- a/src/test/regress/pg_regress_main.c
+++ b/src/test/regress/pg_regress_main.c
@@ -78,11 +78,11 @@ psql_start_test(const char *testname,
* against different AMs without unnecessary differences.
*/
offset += snprintf(psql_cmd + offset, sizeof(psql_cmd) - offset,
- "\"%s%spsql\" -X -a -q -d \"%s\" -v %s < \"%s\" > \"%s\" 2>&1",
+ "\"%s%spsql\" -X -a -q -d \"%s\" %s < \"%s\" > \"%s\" 2>&1",
bindir ? bindir : "",
bindir ? "/" : "",
dblist->str,
- "HIDE_TABLEAM=\"on\"",
+ "-v HIDE_TABLEAM=on -v HIDE_COMPRESSAM=on",
infile,
outfile);
if (offset >= sizeof(psql_cmd))
--
2.17.0
--YZ5djTAD1cGYuMQK
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v24-0003-Add-default_toast_compression-GUC.patch"
^ permalink raw reply [nested|flat] 5+ messages in thread
* PSQL error: total cell count of XXX exceeded
@ 2023-08-26 03:29 Hongxu Ma <[email protected]>
2023-08-26 04:09 ` Re: PSQL error: total cell count of XXX exceeded David G. Johnston <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Hongxu Ma @ 2023-08-26 03:29 UTC (permalink / raw)
To: pgsql-hackers
Hi Hackers,
When I tried to select a big amount of rows, psql complains a error "Cannot add cell to table content: total cell count of 905032704 exceeded."
Here are the reproduce steps:
```
interma=# select version();
version
-----------------------------------------------------------------------------------------
PostgreSQL 12.13 on x86_64-pc-linux-gnu, compiled by gcc (Debian 8.3.0-6) 8.3.0, 64-bit
(1 row)
interma=# create table t26(a int,b int,c int,d int,e int,f int,g int,h int,i int,j int,k int,l int,m int,n int,o int,p int,q int,r int,s int,t int ,u int,v int,w int,x int,y int,z int);
CREATE TABLE
interma=# insert into t26 select generate_series(1,200000000);
INSERT 0 200000000
interma=# select * from t26;
Cannot add cell to table content: total cell count of 905032704 exceeded.
```
I checked the related code, and root cause is clear:
```
// in printTableAddCell()
if (content->cellsadded >= content->ncolumns * content->nrows)
report this error and exit
// cellsadded is long type, but ncolumns and nrows are int
// so, it's possible overflow the int value here.
// using a test program to verify:
int rows = 200000000;
int cols = 26;
printf("%d*%d = %d\n", rows,cols, rows*cols);
output:
2,0000,0000*26 = 9,0503,2704 // overflow and be truncated into int value here
```
Based on it, I think it's a bug. We should use long for ncolumns and nrows and give a more obvious error message here.
My version is 12.13, and I think the latest code also exists this issue: issue: https://github.com/postgres/postgres/blob/1a4fd77db85abac63e178506335aee74625f6499/src/fe_utils/prin...
Any thoughts? or some other hidden reasons?
Thanks.
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: PSQL error: total cell count of XXX exceeded
2023-08-26 03:29 PSQL error: total cell count of XXX exceeded Hongxu Ma <[email protected]>
@ 2023-08-26 04:09 ` David G. Johnston <[email protected]>
2023-08-26 11:19 ` Re: PSQL error: total cell count of XXX exceeded Hongxu Ma <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: David G. Johnston @ 2023-08-26 04:09 UTC (permalink / raw)
To: Hongxu Ma <[email protected]>; +Cc: pgsql-hackers
On Friday, August 25, 2023, Hongxu Ma <[email protected]> wrote:
>
>
> When I tried to select a big amount of rows, psql complains a error "Cannot
> add cell to table content: total cell count of 905032704 exceeded."
>
> We should use long for ncolumns and nrows and give a more obvious error
> message here.
>
> Any thoughts? or some other hidden reasons?
>
9 millions cells seems more than realistic a limit for a psql query result
output. In any case it isn’t a bug, the code demonstrates that fact by
producing an explicit error.
I wouldn’t be adverse to an improved error message, and possibly
documenting said limit.
David J.
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: PSQL error: total cell count of XXX exceeded
2023-08-26 03:29 PSQL error: total cell count of XXX exceeded Hongxu Ma <[email protected]>
2023-08-26 04:09 ` Re: PSQL error: total cell count of XXX exceeded David G. Johnston <[email protected]>
@ 2023-08-26 11:19 ` Hongxu Ma <[email protected]>
2023-09-11 06:50 ` Re: PSQL error: total cell count of XXX exceeded Hongxu Ma <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Hongxu Ma @ 2023-08-26 11:19 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; +Cc: pgsql-hackers
Thank you David.
From the code logic, I don't think this check is meant to check the limit:
If it enters the double-loop (cont.nrows * cont.ncolumns) in printQuery(), the check should be always false (except overflow happened). So, if want to check the limit, we could have done this check before the double-loop: just checking PGresult and reports error earlier.
> I wouldn’t be adverse to an improved error message, and possibly documenting said limit.
Agreed with you, current error message may even report a negative value, it's very confusing for user. It's better to introduce a limit here. Or using a bigger integer type (e.g. long) for them, but it's also have the theoretical upbound.
Thanks.
________________________________
From: David G. Johnston <[email protected]>
Sent: Saturday, August 26, 2023 12:09
To: Hongxu Ma <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: PSQL error: total cell count of XXX exceeded
On Friday, August 25, 2023, Hongxu Ma <[email protected]<mailto:[email protected]>> wrote:
When I tried to select a big amount of rows, psql complains a error "Cannot add cell to table content: total cell count of 905032704 exceeded."
We should use long for ncolumns and nrows and give a more obvious error message here.
Any thoughts? or some other hidden reasons?
9 millions cells seems more than realistic a limit for a psql query result output. In any case it isn’t a bug, the code demonstrates that fact by producing an explicit error.
I wouldn’t be adverse to an improved error message, and possibly documenting said limit.
David J.
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: PSQL error: total cell count of XXX exceeded
2023-08-26 03:29 PSQL error: total cell count of XXX exceeded Hongxu Ma <[email protected]>
2023-08-26 04:09 ` Re: PSQL error: total cell count of XXX exceeded David G. Johnston <[email protected]>
2023-08-26 11:19 ` Re: PSQL error: total cell count of XXX exceeded Hongxu Ma <[email protected]>
@ 2023-09-11 06:50 ` Hongxu Ma <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Hongxu Ma @ 2023-09-11 06:50 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; +Cc: pgsql-hackers
I created a patch to fix it.
Really appreciate to anyone can help to review it.
Thanks.
________________________________
From: Hongxu Ma <[email protected]>
Sent: Saturday, August 26, 2023 19:19
To: David G. Johnston <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: PSQL error: total cell count of XXX exceeded
Thank you David.
From the code logic, I don't think this check is meant to check the limit:
If it enters the double-loop (cont.nrows * cont.ncolumns) in printQuery(), the check should be always false (except overflow happened). So, if want to check the limit, we could have done this check before the double-loop: just checking PGresult and reports error earlier.
> I wouldn’t be adverse to an improved error message, and possibly documenting said limit.
Agreed with you, current error message may even report a negative value, it's very confusing for user. It's better to introduce a limit here. Or using a bigger integer type (e.g. long) for them, but it's also have the theoretical upbound.
Thanks.
________________________________
From: David G. Johnston <[email protected]>
Sent: Saturday, August 26, 2023 12:09
To: Hongxu Ma <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: PSQL error: total cell count of XXX exceeded
On Friday, August 25, 2023, Hongxu Ma <[email protected]<mailto:[email protected]>> wrote:
When I tried to select a big amount of rows, psql complains a error "Cannot add cell to table content: total cell count of 905032704 exceeded."
We should use long for ncolumns and nrows and give a more obvious error message here.
Any thoughts? or some other hidden reasons?
9 millions cells seems more than realistic a limit for a psql query result output. In any case it isn’t a bug, the code demonstrates that fact by producing an explicit error.
I wouldn’t be adverse to an improved error message, and possibly documenting said limit.
David J.
Attachments:
[application/octet-stream] 0001-Using-long-type-in-printTableAddCell.patch (1.6K, ../../TYBP286MB03510C21D207F77D80CF6235B4F2A@TYBP286MB0351.JPNP286.PROD.OUTLOOK.COM/3-0001-Using-long-type-in-printTableAddCell.patch)
download | inline diff:
From 1e3368895e72cc1ab59efecc6b5e93a7752608c7 Mon Sep 17 00:00:00 2001
From: interma <[email protected]>
Date: Mon, 11 Sep 2023 14:42:14 +0800
Subject: [PATCH] Using long type in printTableAddCell() to prevent int
overflow
---
src/fe_utils/print.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c
index 7af1ccb6b5..4d6a9e1d4f 100644
--- a/src/fe_utils/print.c
+++ b/src/fe_utils/print.c
@@ -3249,15 +3249,17 @@ void
printTableAddCell(printTableContent *const content, char *cell,
const bool translate, const bool mustfree)
{
+ long product;
#ifndef ENABLE_NLS
(void) translate; /* unused parameter */
#endif
-
- if (content->cellsadded >= content->ncolumns * content->nrows)
+ /* product of cols and rows, using long type to prevent int overflow */
+ product = (long)content->ncolumns * (long)content->nrows;
+ if (content->cellsadded >= product)
{
fprintf(stderr, _("Cannot add cell to table content: "
- "total cell count of %d exceeded.\n"),
- content->ncolumns * content->nrows);
+ "total cell count of %ld exceeded, cells added: %ld.\n"),
+ product, content->cellsadded);
exit(EXIT_FAILURE);
}
@@ -3273,7 +3275,7 @@ printTableAddCell(printTableContent *const content, char *cell,
{
if (content->cellmustfree == NULL)
content->cellmustfree =
- pg_malloc0((content->ncolumns * content->nrows + 1) * sizeof(bool));
+ pg_malloc0((product + 1) * sizeof(bool));
content->cellmustfree[content->cellsadded] = true;
}
--
2.39.2 (Apple Git-143)
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2023-09-11 06:50 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-14 02:48 [PATCH v24 2/7] psql: Add HIDE_COMPRESSAM for regress testing Justin Pryzby <[email protected]>
2023-08-26 03:29 PSQL error: total cell count of XXX exceeded Hongxu Ma <[email protected]>
2023-08-26 04:09 ` Re: PSQL error: total cell count of XXX exceeded David G. Johnston <[email protected]>
2023-08-26 11:19 ` Re: PSQL error: total cell count of XXX exceeded Hongxu Ma <[email protected]>
2023-09-11 06:50 ` Re: PSQL error: total cell count of XXX exceeded Hongxu Ma <[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