On Tue, Jul 11, 2023 at 07:35:43AM +0900, Michael Paquier wrote:
> I still don't think that we need both methods based on these numbers,
> but there may be more opinions about that? Are people OK if this open
> item is discarded?
Hearing nothing about this point, removed from the open item list,
On 11/7/2023 12:35, Michael Paquier wrote:
On Tue, Jul 11, 2023 at 12:29:29PM +0700, Andrey Lepikhov wrote:
I vote for only one method based on a query tree structure.
Noted
BTW, did you think about different algorithms of queryId generation?
Not really, except if you are referring to the
On Tue, Jul 11, 2023 at 12:29:29PM +0700, Andrey Lepikhov wrote:
> I vote for only one method based on a query tree structure.
Noted
> BTW, did you think about different algorithms of queryId generation?
Not really, except if you are referring to the possibility of being
able to handle different
On 11/7/2023 05:35, Michael Paquier wrote:
On Mon, Jan 30, 2023 at 11:48:45AM +0100, Peter Eisentraut wrote:
On 27.01.23 03:59, Michael Paquier wrote:
At the end, that would be unnoticeable for the average user, I guess,
but here are the numbers I get on my laptop :)
Personally, I think we do
On Mon, Jan 30, 2023 at 11:48:45AM +0100, Peter Eisentraut wrote:
> On 27.01.23 03:59, Michael Paquier wrote:
>> At the end, that would be unnoticeable for the average user, I guess,
>> but here are the numbers I get on my laptop :)
>
> Personally, I think we do not want the two jumble methods in
On Fri, Feb 10, 2023 at 04:40:08PM -0500, Tom Lane wrote:
> v2 looks good to me as far as it goes.
Thanks. I have applied that after an extra lookup.
> I agree these other questions deserve a separate look.
Okay, I may be able to come back to that. Another point is that we
need to do a better
Michael Paquier writes:
> Okay, understood. Following this string of thoughts, I am a bit
> surprised for two cases, though:
> - PartitionPruneStep.
> - Plan.
> Both are abstract and both are marked with no_equal. I guess that
> applying no_query_jumble to both of them is fine, and that's what y
On Thu, Feb 09, 2023 at 06:12:50PM -0500, Tom Lane wrote:
> I'm okay with the pathnodes.h changes --- although surely you don't need
> changes like this:
>
> - pg_node_attr(abstract)
> + pg_node_attr(abstract, no_query_jumble)
>
> "abstract" should already imply "no_query_jumble".
Okay,
Michael Paquier writes:
> Tom, did you get a chance to look at what is proposed here and expand
> the use of query_jumble_ignore in the definitions of the nodes rather
> than have an enforced per-file policy in gen_node_support.pl?
Sorry, didn't look at it before.
I'm okay with the pathnodes.h c
On Wed, Feb 08, 2023 at 03:47:51PM +0900, Michael Paquier wrote:
> This one was intentional to let extensions play with jumbling of such
> nodes, but perhaps you are right that it makes little sense at this
> stage. If there is an ask for it later, though.. Using
> shared_preload_libraries = pg_s
On Tue, Feb 07, 2023 at 11:01:03PM -0800, Andres Freund wrote:
> Given that we already pay the price of multiple regress runs, and that
> jumbling is now really a core feature, perhaps we should enable
> pg_stat_statements in pg_upgrade or 027_stream_regress.pl? I'd hope it
> wouldn't add a meaning
Hi,
On 2023-02-08 15:47:51 +0900, Michael Paquier wrote:
> This one was intentional to let extensions play with jumbling of such
> nodes, but perhaps you are right that it makes little sense at this
> stage. If there is an ask for it later, though.. Using
> shared_preload_libraries = pg_stat_sta
On Tue, Feb 07, 2023 at 05:32:07PM -0500, Tom Lane wrote:
> I have just noticed that this patch is generating useless jumbling
> code for node types such as Path nodes and other planner infrastructure
> nodes. That no doubt contributes to the miserable code coverage rating
> for queryjumblefuncs.*
Michael Paquier writes:
> With all that in mind, I have spent my day polishing that and doing a
> close lookup, and the patch has been applied. Thanks a lot!
I have just noticed that this patch is generating useless jumbling
code for node types such as Path nodes and other planner infrastructure
On Sun, Feb 05, 2023 at 07:40:57PM +0900, Michael Paquier wrote:
> Comments or objections are welcome, of course.
Done this part.
> (FWIW, I'd like to think that there is an argument to normalize the
> A_Const nodes for a portion of the DDL queries, by ignoring their
> values in the query jumblin
On Tue, Jan 31, 2023 at 03:40:56PM +0900, Michael Paquier wrote:
> With all that in mind, I have spent my day polishing that and doing a
> close lookup, and the patch has been applied. Thanks a lot!
While working on a different patch, I have noticed a small issue in
the way the jumbling happens f
On Mon, Jan 30, 2023 at 11:48:45AM +0100, Peter Eisentraut wrote:
> I'm going to set this thread as "Ready for Committer". Either wait a bit
> for more feedback on this topic, or just go ahead with either solution. We
> can leave it as a semi-open item for reconsideration later.
All the measurem
On Mon, Jan 30, 2023 at 11:46:06AM +0100, Peter Eisentraut wrote:
> Either that or src/backend/nodes/README.
The README holds nothing about the node attributes currently, so
nodes.h feels most adapted here at the end..
Thanks,
--
Michael
signature.asc
Description: PGP signature
On 27.01.23 03:59, Michael Paquier wrote:
At the end, that would be unnoticeable for the average user, I guess,
but here are the numbers I get on my laptop :)
Personally, I think we do not want the two jumble methods in parallel.
Maybe there are other opinions.
I'm going to set this thread as
On 27.01.23 04:07, Michael Paquier wrote:
On Thu, Jan 26, 2023 at 09:39:05AM +0100, Peter Eisentraut wrote:
There are a couple of repetitive comments, like "typmod and collation
information are irrelevant for the query jumbling". This applies to all
nodes, so we don't need to repeat it for a nu
On Fri, Jan 27, 2023 at 11:59:47AM +0900, Michael Paquier wrote:
> Using that, I can compile the following results for various cases (-O2
> and compute_query_id=on):
> query | mode | iterations | avg_runtime_ns |
> avg_jumble_ns
> -++--
On Tue, Jan 24, 2023 at 03:57:56PM +0900, Michael Paquier wrote:
> Still, my plan here is to enforce the loading of
> pg_stat_statements with compute_query_id = regress and
> utility_query_id = jumble (if needed) in a new buildfarm machine,
Actually, about this specific point, I have been able to
On Thu, Jan 26, 2023 at 09:39:05AM +0100, Peter Eisentraut wrote:
> There are a couple of repetitive comments, like "typmod and collation
> information are irrelevant for the query jumbling". This applies to all
> nodes, so we don't need to repeat it for a number of nodes (and then not
> mention i
On Thu, Jan 26, 2023 at 09:37:13AM +0100, Peter Eisentraut wrote:
> Ok, the documentation make sense now. I wonder what the performance impact
> is. Probably, nobody cares about microoptimizing CREATE TABLE statements.
> But BEGIN/COMMIT could matter. However, whatever you do in between the
> BE
On 25.01.23 01:08, Michael Paquier wrote:
On Tue, Jan 24, 2023 at 03:57:56PM +0900, Michael Paquier wrote:
Makes sense. That would be my intention if 0004 is the most
acceptable and splitting things makes things a bit easier to review.
There was a silly mistake in 0004 where the jumbling code
On 24.01.23 07:57, Michael Paquier wrote:
For the 0004 patch, it should be documented why one would want one behavior
or the other. That's totally unclear right now.
I am not 100% sure whether we should have this part at the end, but as
an exit path in case somebody complains about the extra lo
On Tue, Jan 24, 2023 at 03:57:56PM +0900, Michael Paquier wrote:
> Makes sense. That would be my intention if 0004 is the most
> acceptable and splitting things makes things a bit easier to review.
There was a silly mistake in 0004 where the jumbling code relied on
compute_query_id rather than ut
On Mon, Jan 23, 2023 at 02:27:13PM +0100, Peter Eisentraut wrote:
> A couple of small fixes are attached.
Thanks.
> There is something weird in _jumbleNode(). There are two switch
> (nodeTag(expr)) statements. Maybe that's intentional, but then it should be
> commented better, because now it lo
On 21.01.23 04:35, Michael Paquier wrote:
I'll read the 0003 again more carefully. I haven't studied the new 0004
yet.
Thanks, again. Rebased version attached.
A couple of small fixes are attached.
There is something weird in _jumbleNode(). There are two switch
(nodeTag(expr)) statements
On Fri, Jan 20, 2023 at 11:56:00AM +0100, Peter Eisentraut wrote:
> In your 0001 patch, most of the comment reformattings for location fields
> are no longer needed, so you should undo those.
>
> The 0002 patch looks good.
Okay, I have gone through these two again and applied what I had.
0001 has
On 20.01.23 05:35, Michael Paquier wrote:
On Thu, Jan 19, 2023 at 09:42:03AM +0100, Peter Eisentraut wrote:
I see that in the 0003 patch, most location fields now have an explicit
markup with query_jumble_ignore. I thought we had previously resolved to
consider location fields to be automatical
On Thu, Jan 19, 2023 at 09:42:03AM +0100, Peter Eisentraut wrote:
> I see that in the 0003 patch, most location fields now have an explicit
> markup with query_jumble_ignore. I thought we had previously resolved to
> consider location fields to be automatically ignored unless explicitly
> included
On Thu, Jan 19, 2023 at 09:42:03AM +0100, Peter Eisentraut wrote:
> I see that in the 0003 patch, most location fields now have an explicit
> markup with query_jumble_ignore. I thought we had previously resolved to
> consider location fields to be automatically ignored unless explicitly
> included
On 18.01.23 08:04, Michael Paquier wrote:
On Tue, Jan 17, 2023 at 04:52:28PM +0900, Michael Paquier wrote:
On Tue, Jan 17, 2023 at 08:43:44AM +0100, Peter Eisentraut wrote:
Ok, I understand now, and I agree with this approach over the opposite. I
was confused because the snippet you showed abov
On Tue, Jan 17, 2023 at 04:52:28PM +0900, Michael Paquier wrote:
> On Tue, Jan 17, 2023 at 08:43:44AM +0100, Peter Eisentraut wrote:
>> Ok, I understand now, and I agree with this approach over the opposite. I
>> was confused because the snippet you showed above used "jumble_ignore", but
>> your pa
On Tue, Jan 17, 2023 at 08:43:44AM +0100, Peter Eisentraut wrote:
> Ok, I understand now, and I agree with this approach over the opposite. I
> was confused because the snippet you showed above used "jumble_ignore", but
> your patch is correct as it uses "jumble_location".
Okay. I'll refresh the
On 17.01.23 04:48, Michael Paquier wrote:
Anyway, when it comes to the location, another thing that can be
considered here would be to require a node-level flag for the nodes on
which we want to track the location. This overlaps a bit with the
fields satisfying "($t eq 'int' && $f =~ 'location$'
On Mon, Jan 16, 2023 at 03:13:35PM +0100, Peter Eisentraut wrote:
> On 13.01.23 08:54, Michael Paquier wrote:
>> Using a "jumble_ignore" flag is equally invasive to using an
>> "jumble_include" flag for each field, I am afraid, as the number of
>> fields in the nodes included in jumbles is pretty e
On 13.01.23 08:54, Michael Paquier wrote:
Using a "jumble_ignore" flag is equally invasive to using an
"jumble_include" flag for each field, I am afraid, as the number of
fields in the nodes included in jumbles is pretty equivalent to the
number of fields ignored. I tend to prefer the approach o
On Sat, Jan 07, 2023 at 07:37:49AM +0100, Peter Eisentraut wrote:
> On 07.12.22 08:56, Michael Paquier wrote:
>> The location of the Nodes is quite invasive because we only care about
>> that for T_Const now in the query jumbling, and this could be
>> compensated with a third pg_node_attr() that tr
On Sat, Jan 07, 2023 at 07:37:49AM +0100, Peter Eisentraut wrote:
> The generation script already has a way to identify location fields, by ($t
> eq 'int' && $f =~ 'location$'), so you could use that as well.
I recall that some of the nodes may need renames to map with this
choice. That could be
On 07.12.22 08:56, Michael Paquier wrote:
The location of the Nodes is quite invasive because we only care about
that for T_Const now in the query jumbling, and this could be
compensated with a third pg_node_attr() that tracks for the "int
location" of a Node whether it should participate in the
On Wed, 7 Dec 2022 at 13:27, Michael Paquier wrote:
>
> Hi all,
>
> This thread is a follow-up of the recent discussion about query
> jumbling with DDL statements, where the conclusion was that we'd want
> to generate all this code automatically for all the nodes:
> https://www.postgresql.org/mess
Hi all,
This thread is a follow-up of the recent discussion about query
jumbling with DDL statements, where the conclusion was that we'd want
to generate all this code automatically for all the nodes:
https://www.postgresql.org/message-id/36e5bffe-e989-194f-85c8-06e7bc88e...@amazon.com
What this
44 matches
Mail list logo
