public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 03/18] pg_restore: "must be specified" and --list
3+ messages / 3 participants
[nested] [flat]
* [PATCH 03/18] pg_restore: "must be specified" and --list
@ 2020-12-06 04:43 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Justin Pryzby @ 2020-12-06 04:43 UTC (permalink / raw)
This was discussed here, but the idea got lost.
https://www.postgresql.org/message-id/flat/20190612170201.GA11881%40alvherre.pgsql#2984347ab074e6f19...
---
src/bin/pg_dump/pg_restore.c | 2 +-
src/bin/pg_dump/t/001_basic.pl | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 589b4aed53..f6e6e41329 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -305,7 +305,7 @@ main(int argc, char **argv)
/* Complain if neither -f nor -d was specified (except if dumping TOC) */
if (!opts->cparams.dbname && !opts->filename && !opts->tocSummary)
{
- pg_log_error("one of -d/--dbname and -f/--file must be specified");
+ pg_log_error("one of -d/--dbname, -f/--file, or -l/--list must be specified");
exit_nicely(1);
}
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 083fb3ad08..8280914c2a 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -63,8 +63,8 @@ command_fails_like(
command_fails_like(
['pg_restore'],
- qr{\Qpg_restore: error: one of -d/--dbname and -f/--file must be specified\E},
- 'pg_restore: error: one of -d/--dbname and -f/--file must be specified');
+ qr{\Qpg_restore: error: one of -d/--dbname, -f/--file, or -l/--list must be specified\E},
+ 'pg_restore: error: one of -d/--dbname, -f/--file, or -l/--list must be specified');
command_fails_like(
[ 'pg_restore', '-s', '-a', '-f -' ],
--
2.17.0
--lc9FT7cWel8HagAv
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0004-typos-in-master.patch"
^ permalink raw reply [nested|flat] 3+ messages in thread
* Wrong results due to missing quals
@ 2023-05-24 11:19 Richard Guo <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Richard Guo @ 2023-05-24 11:19 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Tom Lane <[email protected]>
Testing with SQLancer reports a wrong results issue on master and I
reduced it to the repro query below.
create table t (a int, b int);
explain (costs off)
select * from t t1 left join
(t t2 left join t t3 full join t t4 on false on false)
left join t t5 on t2.a = t5.a
on t2.b = 1;
QUERY PLAN
--------------------------------------------------
Nested Loop Left Join
-> Seq Scan on t t1
-> Materialize
-> Nested Loop Left Join
-> Nested Loop Left Join
Join Filter: false
-> Seq Scan on t t2
Filter: (b = 1)
-> Result
One-Time Filter: false
-> Materialize
-> Seq Scan on t t5
(12 rows)
So the qual 't2.a = t5.a' is missing.
I looked into it and found that both clones of this joinqual are
rejected by clause_is_computable_at, because their required_relids do
not include the outer join of t2/(t3/t4), and meanwhile include nullable
rels of this outer join.
I think the root cause is that, as Tom pointed out in [1], we're not
maintaining required_relids very accurately. In b9c755a2, we make
clause_is_computable_at test required_relids for clone clauses. I think
this is how this issue sneaks in.
To fix it, it seems to me that the ideal way would be to always compute
accurate required_relids. But I'm not sure how difficult it is.
[1] https://www.postgresql.org/message-id/395264.1684698283%40sss.pgh.pa.us
Thanks
Richard
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Wrong results due to missing quals
@ 2023-05-24 15:10 Tom Lane <[email protected]>
parent: Richard Guo <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Tom Lane @ 2023-05-24 15:10 UTC (permalink / raw)
To: Richard Guo <[email protected]>; +Cc: pgsql-hackers
Richard Guo <[email protected]> writes:
> So the qual 't2.a = t5.a' is missing.
Ugh.
> I looked into it and found that both clones of this joinqual are
> rejected by clause_is_computable_at, because their required_relids do
> not include the outer join of t2/(t3/t4), and meanwhile include nullable
> rels of this outer join.
> I think the root cause is that, as Tom pointed out in [1], we're not
> maintaining required_relids very accurately. In b9c755a2, we make
> clause_is_computable_at test required_relids for clone clauses. I think
> this is how this issue sneaks in.
Yeah. I'm starting to think that b9c755a2 took the wrong approach.
Really, required_relids is about making sure that a qual isn't
evaluated "too low", before all necessary joins have been formed. But
clause_is_computable_at is charged with making sure we don't evaluate
it "too high", after some incompatible join has been formed. There's
no really good reason to suppose that required_relids can serve both
purposes, even if it were computed perfectly accurately (and what is
perfect, anyway?).
So right now I'm playing with the idea of reverting the change in
clause_is_computable_at and seeing how else we can fix the previous
bug. Don't have anything to show yet, but one thought is that maybe
deconstruct_distribute_oj_quals needs to set up clause_relids for
clone clauses differently. Another idea is that maybe we need another
RestrictInfo field that's directly a set of OJ relids that this clause
can't be applied above. That'd reduce clause_is_computable_at to
basically a bms_intersect test which would be nice speed-wise. The
space consumption could be annoying, but I'm thinking that we might
only have to populate the field in clone clauses, which would
alleviate that issue.
regards, tom lane
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-05-24 15:10 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-12-06 04:43 [PATCH 03/18] pg_restore: "must be specified" and --list Justin Pryzby <[email protected]>
2023-05-24 11:19 Wrong results due to missing quals Richard Guo <[email protected]>
2023-05-24 15:10 ` Re: Wrong results due to missing quals Tom Lane <[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