Hi
> Sergei> PS: my note about comments in tests from my previous review is
> Sergei> actual too.
>
> I changed the comment when committing it.
Great, thank you!
regards, Sergei
> "Sergei" == Sergei Kornilov writes:
Sergei> PS: my note about comments in tests from my previous review is
Sergei> actual too.
I changed the comment when committing it.
--
Andrew (irc:RhodiumToad)
On Thu, Jul 11, 2019 at 9:08 PM Sergei Kornilov wrote:
> Patch is still applied cleanly on HEAD and passes check-world. I think
> ignoring FOR UPDATE clause is incorrect behavior.
With my reviewer hat: I agree.
With my CFM hat: It seems like this patch is ready to land so I have
set it to "Read
Hello
Patch is still applied cleanly on HEAD and passes check-world. I think ignoring
FOR UPDATE clause is incorrect behavior.
PS: my note about comments in tests from my previous review is actual too.
regards, Sergei
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation:not tested
Hello
Patch is applied cleanly, compiles and pass check-world. Has t
> "Tom" == Tom Lane writes:
Tom> +1 for not ignoring rowMarks, but this patch seems like a hack to
Tom> me. Why didn't you just add RowMarkClause as one of the known
Tom> alternative expression node types?
>> Because it's not an expression and nothing anywhere else in the
>> backend tre
On 19/01/2019 18:02, Tom Lane wrote:
> Vik Fearing writes:
>> Does the extension need a version bump for this?
>
> We don't bump its version when we make any other change that affects
> its hash calculation. I don't think this could be back-patched,
> but Andrew wasn't proposing to do so (IIUC).
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> +1 for not ignoring rowMarks, but this patch seems like a hack to
> Tom> me. Why didn't you just add RowMarkClause as one of the known
> Tom> alternative expression node types?
> Because it's not an expression and nothing anywhere else
> "Tom" == Tom Lane writes:
> Andrew Gierth writes:
>> I propose that it should not ignore rowMarks, per the attached patch or
>> something similar.
Tom> +1 for not ignoring rowMarks, but this patch seems like a hack to
Tom> me. Why didn't you just add RowMarkClause as one of the known
Vik Fearing writes:
> Does the extension need a version bump for this?
We don't bump its version when we make any other change that affects
its hash calculation. I don't think this could be back-patched,
but Andrew wasn't proposing to do so (IIUC).
regards, tom lane
Andrew Gierth writes:
> I propose that it should not ignore rowMarks, per the attached patch or
> something similar.
+1 for not ignoring rowMarks, but this patch seems like a hack to me.
Why didn't you just add RowMarkClause as one of the known alternative
expression node types? There's no advan
On 19/01/2019 15:43, Andrew Gierth wrote:
> pg_stat_statements considers a plain select and a select for update to
> be equivalent, which seems quite wrong to me as they will have very
> different performance characteristics due to locking.
>
> The only comment about it in the code is:
>
> /*
pg_stat_statements considers a plain select and a select for update to
be equivalent, which seems quite wrong to me as they will have very
different performance characteristics due to locking.
The only comment about it in the code is:
/* we ignore rowMarks */
I propose that it should not ign
13 matches
Mail list logo
