On Thu, Apr 21, 2022 at 8:38 AM Thomas Munro wrote:
> On Thu, Apr 21, 2022 at 7:35 AM Robert Haas wrote:
> > On Wed, Apr 20, 2022 at 3:08 PM Thomas Munro wrote:
> > > Looks like this somehow broke on a Windows box:
> > >
> > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=202
On Thu, Apr 21, 2022 at 7:35 AM Robert Haas wrote:
> On Wed, Apr 20, 2022 at 3:08 PM Thomas Munro wrote:
> > Looks like this somehow broke on a Windows box:
> >
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2022-04-20%2016%3A34%3A19
>
> So the issue here is that we are run
On Wed, Apr 20, 2022 at 3:08 PM Thomas Munro wrote:
> Looks like this somehow broke on a Windows box:
>
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2022-04-20%2016%3A34%3A19
So the issue here is that we are running this command:
pg_dumpall --exclude-database .*
And on th
On Thu, Apr 21, 2022 at 3:55 AM Robert Haas wrote:
> On Tue, Apr 19, 2022 at 10:20 PM Mark Dilger
> wrote:
> > Looks like most people voted for (B). In support of that option, here are
> > patches for master and REL_14_STABLE. Note that I extended the tests
> > compared to v9, which found a p
On Tue, Apr 19, 2022 at 10:20 PM Mark Dilger
wrote:
> Looks like most people voted for (B). In support of that option, here are
> patches for master and REL_14_STABLE. Note that I extended the tests
> compared to v9, which found a problem that is fixed for v10:
OK, I committed these. I am not
On 4/19/22 16:00, Robert Haas wrote:
On Mon, Apr 18, 2022 at 3:39 PM Mark Dilger
wrote:
Since there hasn't been any agreement on that point, I've just rebased the
patch to apply cleanly against the current master:
This looks OK to me. There may be better ways to do some of it, but
there's no
Justin Pryzby writes:
> On Tue, Apr 19, 2022 at 10:00:01AM -0400, Robert Haas wrote:
>> (A) This is a new feature. Wait for v16.
>> (B) This is a bug fix. Commit it now and back-patch to v14.
>> (C) This is a cleanup that is OK to put into v15 even after feature
>> freeze but since it is a behavio
On Tue, Apr 19, 2022 at 10:00:01AM -0400, Robert Haas wrote:
> (A) This is a new feature. Wait for v16.
> (B) This is a bug fix. Commit it now and back-patch to v14.
> (C) This is a cleanup that is OK to put into v15 even after feature
> freeze but since it is a behavior change we shouldn't back-pa
On Tue, Apr 19, 2022 at 7:00 AM Robert Haas wrote:
> On Mon, Apr 18, 2022 at 3:39 PM Mark Dilger
> wrote:
> > Since there hasn't been any agreement on that point, I've just rebased
> the patch to apply cleanly against the current master:
>
> This looks OK to me. There may be better ways to do so
On Mon, Apr 18, 2022 at 3:39 PM Mark Dilger
wrote:
> Since there hasn't been any agreement on that point, I've just rebased the
> patch to apply cleanly against the current master:
This looks OK to me. There may be better ways to do some of it, but
there's no rule against further improving the c
> On Apr 8, 2022, at 4:11 AM, Robert Haas wrote:
>
> I don't personally see how we're going to come out ahead with that
> approach, but if you or Tom or someone else want to put something
> together, that's fine with me. I'm not stuck on this approach, I just
> don't see how we come out ahead w
On Thu, Apr 7, 2022 at 11:40 PM Greg Stark wrote:
> That doesn't seem to be entirely inconsistent with what Tom describes.
> Instead of "throw an error" the function would return an error and
> possibly some extra info which the caller would use to handle the
> error appropriately.
I don't person
On Thu, 7 Apr 2022 at 22:32, Robert Haas wrote:
>
> On Thu, Apr 7, 2022 at 7:41 PM Tom Lane wrote:
>
> > Possibly a better idea is to add an enum argument telling the function
> > what to do (parse the whole thing as one name regardless of dots,
> > parse as two names if there's a dot, throw erro
On Thu, Apr 07, 2022 at 10:26:18PM -0400, Robert Haas wrote:
> + pg_log_error("improper relation name (too many dotted names):
> %s", pattern);
>
> Come to think of it, maybe the error text there could stand some
> bikeshedding, but AFAICS
AFAICT the error text deliberately matches
On Thu, Apr 7, 2022 at 7:41 PM Tom Lane wrote:
> Mark Dilger writes:
> > The patch submitted changes processSQLNamePattern() to return a dot count
> > by reference. It's up to the caller to decide whether to raise an error.
> > If you pass in no schemavar, and you get back dotcnt=2, you know
Mark Dilger writes:
> The patch submitted changes processSQLNamePattern() to return a dot count by
> reference. It's up to the caller to decide whether to raise an error. If
> you pass in no schemavar, and you get back dotcnt=2, you know it parsed it as
> a two part pattern, and you can pg_fa
> On Apr 7, 2022, at 3:37 PM, Tom Lane wrote:
>
>>
>> I don't know whether that's a bug fix for the existing code or some
>> new bit of functionality that \dconfig requires and nothing else
>> needs.
>
> Well, \dconfig needs it because it would like foo.bar to get processed
> as just a name.
Robert Haas writes:
> I still have a vague feeling that there's probably some way of doing
> this better, but had more or less resolved to commit this patch as is
> anyway and had that all queued up. But then I had to go to a meeting
> and when I came out I discovered that Tom had done this:
Sorr
On Wed, Apr 6, 2022 at 12:07 PM Mark Dilger
wrote:
> I was able to clean up the "if (left && want_literal_dbname)" stuff, though.
I still have a vague feeling that there's probably some way of doing
this better, but had more or less resolved to commit this patch as is
anyway and had that all queu
> On Mar 29, 2022, at 8:20 AM, Robert Haas wrote:
>
> In describe.c, why are the various describeWhatever functions
> returning true when validateSQLNamePattern returns false? It seems to
> me that they should return false. That would cause exec_command_d() to
> set status = PSQL_CMD_ERROR, whi
On Fri, Mar 25, 2022 at 3:42 PM Mark Dilger
wrote:
> I think your change is fine, so I've rolled it into this next patch.
OK, cool. Here are some more comments.
In describe.c, why are the various describeWhatever functions
returning true when validateSQLNamePattern returns false? It seems to
me
> On Mar 22, 2022, at 11:04 AM, Robert Haas wrote:
>
> This patch adds three new arguments to processSQLNamePattern() and
> documents one of them. It adds three new parameters to
> patternToSQLRegex() as well, and documents none of them.
This next patch adds the missing comments.
> I think th
On Mon, Mar 21, 2022 at 9:32 PM Mark Dilger
wrote:
> [ new patch version ]
This patch adds three new arguments to processSQLNamePattern() and
documents one of them. It adds three new parameters to
patternToSQLRegex() as well, and documents none of them. I think that
the text of the comment might
> On Mar 21, 2022, at 6:12 PM, Andres Freund wrote:
>
> Needs another one: http://cfbot.cputube.org/patch_37_3367.log
>
> Marked as waiting-on-author.
v6-0001-Reject-patterns-with-too-many-parts-or-wrong-db.patch
Description: Binary data
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.
On 2022-01-26 09:04:15 -0800, Mark Dilger wrote:
> Also, rebased as necessary:
Needs another one: http://cfbot.cputube.org/patch_37_3367.log
Marked as waiting-on-author.
Greetings,
Andres Freund
On Tue, Mar 15, 2022 at 12:31 PM Mark Dilger
wrote:
>
> > On Mar 15, 2022, at 12:27 PM, Robert Haas wrote:
> >
> > - Justin Pryzby, who originally discovered the problem, prefers the
> > same behavior that I prefer long-term, but thinks Tom's behavior is
> > better than doing nothing.
> > - Mark
On Wed, Oct 13, 2021 at 11:54:26AM -0500, Justin Pryzby wrote:
> It seems unfortunate if names from log messages qualified with datname were
> now
> rejected. Like this one:
>
> | automatic analyze of table "ts.child.cdrs_2021_10_12"...
Mark mentioned this "log message" use case in his proposed
> On Mar 15, 2022, at 12:27 PM, Robert Haas wrote:
>
> - Justin Pryzby, who originally discovered the problem, prefers the
> same behavior that I prefer long-term, but thinks Tom's behavior is
> better than doing nothing.
> - Mark Dilger, Isaac Moreland, Garick Hamlin, Alvaro Herrera, and
> Ju
Continuing my pass through the "bug fixes" section of the CommitFest,
I came upon this patch, which is contested. Here is my attempt to
summarize where things stand. As I understand it:
- Tom wants to revert to the previous behavior of accepting arbitrary
garbage, so that \d slkgjskld.jgdsjhgjklsd
> On Jan 17, 2022, at 1:54 PM, Robert Haas wrote:
>
> + * dotcnt: how many separators were parsed from the pattern, by reference.
> + * Can be NULL.
>
> But then:
>
> +Assert(dotcnt != NULL);
Removed the "Can be NULL" part, as that use case doesn't make sense. The
caller should always
On Mon, Jan 17, 2022 at 1:06 PM Mark Dilger
wrote:
> > On Jan 15, 2022, at 12:28 AM, Julien Rouhaud wrote:
> > Could you send a rebased version?
> Yes. Here it is:
This is not a full review, but I just noticed that:
+ * dotcnt: how many separators were parsed from the pattern, by reference.
+
> On Jan 15, 2022, at 12:28 AM, Julien Rouhaud wrote:
>
> Could you send a rebased version?
Yes. Here it is:
v4-0001-Reject-patterns-with-too-many-parts-or-wrong-db.patch
Description: Binary data
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi,
On Tue, Dec 21, 2021 at 10:58:39AM -0800, Mark Dilger wrote:
>
> Rebased patch attached:
This version doesn't apply anymore:
http://cfbot.cputube.org/patch_36_3367.log
=== Applying patches on top of PostgreSQL commit ID
5513dc6a304d8bda114004a3b906cc6fde5d6274 ===
=== applying patch
./v3-0
Rebased patch attached:
v3-0001-Reject-patterns-with-too-many-parts-or-wrong-db.patch
Description: Binary data
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
> On Nov 5, 2021, at 6:59 AM, Tom Lane wrote:
>
> Alvaro Herrera writes:
>> I think it would appropriate to normalize identifiers that are going to
>> be stored in catalogs. As presented, this is a bit ridiculous and I see
>> no reason to continue to support it.
>
> If we had any sort of co
On Fri, Nov 5, 2021 at 9:59 AM Tom Lane wrote:
> In any case, that seems quite orthogonal to the question of how to treat
> names with too many dots in them. Considering we are three days out from
> freezing 14.1, I think it is time to stop the meandering discussion and
> fix it. And by "fix", I
Alvaro Herrera writes:
> I think it would appropriate to normalize identifiers that are going to
> be stored in catalogs. As presented, this is a bit ridiculous and I see
> no reason to continue to support it.
If we had any sort of convention about the encoding of identifiers stored
in shared ca
On 2021-Oct-20, Mark Dilger wrote:
> I tried testing how this plays out by handing `createdb` the name é
> (U+00E9 "LATIN SMALL LETTER E WITH ACCUTE") and then again the name é
> (U+0065 "LATIN SMALL LETTER E" followed by U+0301 "COMBINING ACCUTE
> ACCENT".) That results in two distinct databases
> On Nov 4, 2021, at 6:37 AM, Hamlin, Garick L wrote:
>
>> If we pass the dots through to the POSIX regular expression, we can
>> only do that either for the table name or the schema name, not both -
>> either the first or last dot must mark the boundary between the two.
>> That means that you
On Wed, Oct 13, 2021 at 09:24:53AM -0400, Robert Haas wrote:
> > Splitting the pattern on all the dots and throwing away any additional
> > leftmost fields is a bug, ...
>
> I also agree with you right up to here.
>
> > and when you stop doing that, passing additional dots through to the POSIX
>
> On Nov 3, 2021, at 12:07 PM, Tom Lane wrote:
>
> Mark Dilger writes:
>> [ v1-0001-Reject-patterns-with-too-many-parts-or-wrong-db.patch ]
>
> This needs a rebase over the recent renaming of our Perl test modules.
> (Per the cfbot, so do several of your other pending patches.)
>
>
Mark Dilger writes:
> [ v1-0001-Reject-patterns-with-too-many-parts-or-wrong-db.patch ]
This needs a rebase over the recent renaming of our Perl test modules.
(Per the cfbot, so do several of your other pending patches.)
regards, tom lane
> On Oct 13, 2021, at 1:43 PM, Mark Dilger wrote:
>
> The issue of name parsing impacts pg_dump and pg_dumpall, also. Consider
> what happens with:
>
> pg_dump -t production.critical.secrets > secrets.dump
> dropdb production
>
> In v13, if your default database is "testing", and database "t
On Wed, Oct 13, 2021 at 4:43 PM Mark Dilger
wrote:
> The function where the processing occurs is processSQLNamePattern, which is
> called by pg_dump, pg_dumpall, and psql. All three callers expect
> processSQLNamePattern to append where-clauses to a buffer, not to execute any
> sql of its own.
> On Oct 13, 2021, at 8:43 AM, Robert Haas wrote:
>
> On Wed, Oct 13, 2021 at 10:40 AM Mark Dilger
> wrote:
>> 3a is a bit strange, when considered in the context of patterns. If db1,
>> db2, and db3 all exist and each have a table foo.bar, and psql is connected
>> to db1, how should the c
Greetings,
* Robert Haas (robertmh...@gmail.com) wrote:
> On Wed, Oct 13, 2021 at 12:54 PM Justin Pryzby wrote:
> > It seems unfortunate if names from log messages qualified with datname were
> > now
> > rejected. Like this one:
> >
> > | automatic analyze of table "ts.child.cdrs_2021_10_12"...
On Wed, Oct 13, 2021 at 12:54 PM Justin Pryzby wrote:
> It seems unfortunate if names from log messages qualified with datname were
> now
> rejected. Like this one:
>
> | automatic analyze of table "ts.child.cdrs_2021_10_12"...
That's a good argument, IMHO.
--
Robert Haas
EDB: http://www.ente
On Wed, Oct 13, 2021 at 12:46:27PM -0400, Robert Haas wrote:
> On Tue, Oct 12, 2021 at 12:57 PM Justin Pryzby wrote:
> > I would prefer if it errored if the datname didn't match the current
> > database.
> > After all, it would've helped me to avoid making a confusing problem report.
>
> How wou
On Tue, Oct 12, 2021 at 12:57 PM Justin Pryzby wrote:
> I would prefer if it errored if the datname didn't match the current database.
> After all, it would've helped me to avoid making a confusing problem report.
How would you have felt if it had said something like:
error: argument to \d shoul
On Wed, Oct 13, 2021 at 10:40 AM Mark Dilger
wrote:
> 3a is a bit strange, when considered in the context of patterns. If db1,
> db2, and db3 all exist and each have a table foo.bar, and psql is connected
> to db1, how should the command \d db?.foo.bar behave? We have no problem
> with db1.fo
> On Oct 13, 2021, at 6:24 AM, Robert Haas wrote:
>
>> and when you stop doing that, passing additional dots through to the POSIX
>> regular expression for processing is the most natural thing to do. This is,
>> in fact, how v14 works. It is a bit debatable whether treating the first
>> d
On Tue, Oct 12, 2021 at 5:21 PM Mark Dilger
wrote:
> I wasn't thinking critically enough about how psql handles \d when I accepted
> Justin's initial characterization of the bug. The psql client has never
> thought about the stuff to the left of the schema name as a database name,
> even if so
> On Oct 12, 2021, at 10:18 AM, Mark Dilger
> wrote:
>
> Here is a WIP patch that restores the old behavior, just so you can eyeball
> how large it is. (It passes check-world and I've read it over once, but I'm
> not ready to stand by this as correct quite yet.) I need to add a regression
> On Oct 12, 2021, at 10:54 AM, Robert Haas wrote:
>
> On Tue, Oct 12, 2021 at 1:18 PM Mark Dilger
> wrote:
>> Here is a WIP patch that restores the old behavior, just so you can eyeball
>> how large it is.
>
> I guess that's not that bad. Why did we end up with the behavior that
> the curr
On Tue, Oct 12, 2021 at 1:18 PM Mark Dilger
wrote:
> Here is a WIP patch that restores the old behavior, just so you can eyeball
> how large it is.
I guess that's not that bad. Why did we end up with the behavior that
the current comment describes this way?
"(Additional dots in the name portion
> On Oct 12, 2021, at 10:01 AM, Robert Haas wrote:
>
> On Tue, Oct 12, 2021 at 12:44 PM Peter Geoghegan wrote:
>> You're asking us to imagine a counterfactual. But this counterfactual
>> bug report would have to describe a real practical problem.
>
> Yes. And I think this one should be held
> On Oct 12, 2021, at 10:03 AM, Robert Haas wrote:
>
> On Tue, Oct 12, 2021 at 12:57 PM Justin Pryzby wrote:
>> I think there's an easy answer here that would satisfy everyone; two patches:
>> 0001 to fix the unintentional behavior change;
>> 0002 to reject garbage input: anything with more th
On Tue, Oct 12, 2021 at 12:57 PM Justin Pryzby wrote:
> I think there's an easy answer here that would satisfy everyone; two patches:
> 0001 to fix the unintentional behavior change;
> 0002 to reject garbage input: anything with more than 3 dot-separated
> components, or with 3 components whe
On Tue, Oct 12, 2021 at 12:44 PM Peter Geoghegan wrote:
> You're asking us to imagine a counterfactual. But this counterfactual
> bug report would have to describe a real practical problem.
Yes. And I think this one should be held to the same standard: \d
mydb.myschema.mytable not working is pote
I understand Tom's position to be that the behavior should be changed back,
since it was 1) unintentional; and 2) breaks legitimate use (when the datname
matches current_database).
I think there's an easy answer here that would satisfy everyone; two patches:
0001 to fix the unintentional behavior
On 10/12/21 5:19 PM, Stephen Frost wrote:
> Greetings,
>
> * Robert Haas (robertmh...@gmail.com) wrote:
>> On Tue, Oct 12, 2021 at 10:31 AM Tom Lane wrote:
>>> If the behavior v14 had implemented were "throw an error if the
>>> first word doesn't match the current database name", perhaps nobody
>
On Tue, Oct 12, 2021 at 7:41 AM Robert Haas wrote:
> Oh, give me a break. The previous behavior obviously hasn't been
> tested either, and is broken on its face. If someone *had* complained
> about it, I imagine you would have promptly fixed it and likely
> back-patched the fix, probably in under
Greetings,
* Robert Haas (robertmh...@gmail.com) wrote:
> On Tue, Oct 12, 2021 at 10:31 AM Tom Lane wrote:
> > If the behavior v14 had implemented were "throw an error if the
> > first word doesn't match the current database name", perhaps nobody
> > would have questioned it. But that's not what
On Tue, Oct 12, 2021 at 10:31 AM Tom Lane wrote:
> If the behavior v14 had implemented were "throw an error if the
> first word doesn't match the current database name", perhaps nobody
> would have questioned it. But that's not what we have. It's fairly
> clear that neither you nor Mark thought
> On Oct 12, 2021, at 7:30 AM, Tom Lane wrote:
>
> If the behavior v14 had implemented were "throw an error if the
> first word doesn't match the current database name", perhaps nobody
> would have questioned it. But that's not what we have. It's fairly
> clear that neither you nor Mark thou
Robert Haas writes:
> On Mon, Oct 11, 2021 at 10:33 PM Peter Geoghegan wrote:
>> Being lenient here just doesn't have much downside in practice, as
>> evidenced by the total lack of complaints about that lenience.
> I find it kind of surprising to find everyone agreeing with this
> argument.
If
On Mon, Oct 11, 2021 at 10:33 PM Peter Geoghegan wrote:
> On Mon, Oct 11, 2021 at 7:09 PM Mark Dilger
> wrote:
> > I was just wondering when it might be time to stop being lenient in psql
> > and instead reject malformed identifiers.
>
> I suppose that I probably wouldn't have chosen this behavi
On Mon, Oct 11, 2021 at 7:09 PM Mark Dilger
wrote:
> I was just wondering when it might be time to stop being lenient in psql and
> instead reject malformed identifiers.
I suppose that I probably wouldn't have chosen this behavior in a
green field situation. But Hyrum's law tells us that there a
> On Oct 11, 2021, at 4:49 PM, Tom Lane wrote:
>
> You are attacking a straw man here. To use a period in an identifier,
> you have to double-quote it; that's the same in SQL or \d.
That's a strange argument. If somebody gives an invalid identifier, we
shouldn't assume they know the proper
Mark Dilger writes:
> But since we allow tables and schemas with dotted names in them, I'm
> uncertain what \d foo.bar.baz is really asking. That could be
> "foo.bar"."baz", or "foo"."bar"."baz", or "foo"."bar.baz", or even
> "public"."foo.bar.baz". The old behavior seems a bit dangerous. T
On Mon, 11 Oct 2021 at 19:35, Mark Dilger
wrote:
> But since we allow tables and schemas with dotted names in them, I'm
> uncertain what \d foo.bar.baz is really asking.
>
FWIW, it’s absolutely clear to me that "." is a special character which has
to be quoted in order to be in an identifier.
> On Oct 11, 2021, at 3:37 PM, Tom Lane wrote:
>
>> REL_13_STABLE appears to accept any amount of nonsense you like:
>
> Yeah, I'm pretty sure that the old rule was to just ignore whatever
> appeared in the database-name position. While we could tighten that
> up to insist that it match the
On Mon, Oct 11, 2021 at 02:47:59PM -0700, Mark Dilger wrote:
> > |$ LD_LIBRARY_PATH=tmp_install/usr/local/pgsql/lib/ src/bin/psql/psql -h
> > /tmp regression
> > |psql (15devel)
> > |Type "help" for help.
> > |regression=# \d regresion.public.bit_defaults
> > |Did not find any relation named "regr
Mark Dilger writes:
>> On Oct 11, 2021, at 3:04 PM, Tom Lane wrote:
>> Doesn't work with the correct DB name, either:
>> regression=# \d regression.public.bit_defaults
>> Did not find any relation named "regression.public.bit_defaults".
> REL_13_STABLE appears to accept any amount of nonsense yo
> On Oct 11, 2021, at 3:26 PM, Justin Pryzby wrote:
>
> It looks like before v13 any "datname" prefix was ignored.
The evidence so far suggests that something is broken in v14, but it is less
clear to me what the appropriate behavior is. The v14 psql is rejecting even a
correctly named dat
> On Oct 11, 2021, at 3:04 PM, Tom Lane wrote:
>
> Doesn't work with the correct DB name, either:
>
> regression=# \d public.bit_defaults
> Table "public.bit_defaults"
> Column | Type | Collation | Nullable | Default
> ++-
Mark Dilger writes:
> I can only assume that you are intentionally misspelling "regression" as
> "regresion" (with only one "s") as part of the test. I have not checked if
> that worked before v14, but if it ignored the misspelled database name before
> v14, and it rejects it now, I'm not sure
> On Oct 11, 2021, at 2:24 PM, Justin Pryzby wrote:
>
> This commit broke psql \d datname.nspname.relname
>
> commit 2c8726c4b0a496608919d1f78a5abc8c9b6e0868
> Author: Robert Haas
> Date: Wed Feb 3 13:19:41 2021 -0500
>
>Factor pattern-construction logic out of processSQLNamePattern.
This commit broke psql \d datname.nspname.relname
commit 2c8726c4b0a496608919d1f78a5abc8c9b6e0868
Author: Robert Haas
Date: Wed Feb 3 13:19:41 2021 -0500
Factor pattern-construction logic out of processSQLNamePattern.
...
patternToSQLRegex is a little more general than what is required
79 matches
Mail list logo
