>
I agree that additional testing in this area is valuable. Patch 0001
looks reasonable to me, on a quick read-through. In patch 0002, I
think it would be valuable to also test updating the parent row to
time periods consistent and not consistent with the insert, to confirm
that that behaves correctly.
Regards,
Dean
On Wed, 16 Jul 2025 at 19:23, Dean Rasheed wrote:
>
> On 64-bit Linux with gcc 14.2 and native int128 support disabled I got
> the following results:
>
> Query 1:
> HEAD: 1404.096 ms
> Patch: 992.818 ms
>
> Query 2:
> HEAD: 1498.949 ms
> Patch: 935.6
On Wed, 16 Jul 2025 at 22:22, Nathan Bossart wrote:
>
> Okay, here is a new version of the patch.
>
LGTM.
Regards,
Dean
On Wed, 16 Jul 2025 at 18:26, Nathan Bossart wrote:
>
> On Tue, Jul 15, 2025 at 04:14:44PM +0100, Dean Rasheed wrote:
> > On Tue, 15 Jul 2025 at 04:17, Robert Treat wrote:
> >> On Mon, Jul 14, 2025 at 3:22 PM David G. Johnston
> >> > The pg_overexplain exte
On Wed, 16 Jul 2025 at 10:02, John Naylor wrote:
>
> On Mon, Jun 23, 2025 at 3:01 PM Dean Rasheed wrote:
> > 0005 is the main patch. It adds a few more functions to int128.h and
> > uses them in numeric.c to allow various functions (mainly aggregate
> > functions)
(in
> either one sentence or two sentence format).
>
+1. I find that easier to read.
Something else that's missing is instructions on how to load the
module, which usually follows this first paragraph.
Regards,
Dean
On Tue, 15 Jul 2025 at 04:49, Robert Treat wrote:
>
> On Mon, Jul 14, 2025 at 3:21 AM Dean Rasheed wrote:
> >
> > But it's completely trivial to emulate random(min_date, max_date), just by
> > doing
> >
> > min_date + random(0, max_date - min_date)
On Mon, 14 Jul 2025 at 22:07, Dean Rasheed wrote:
>
> > warning: format ‘%lX’ expects argument of type ‘long unsigned int’,
> > but argument 4 has type ‘int32’ {aka ‘int’} [-Wformat=]
> >
> v4 attached.
v5 attached, fixing some more printf-related compiler warnings, thi
yes, thanks for pointing that out.
(The cfbot reports the same warnings, but you have to scroll through a
lot of output to see them. It would be nice if the commitfest app had
an indicator to show if there were any compiler warnings.)
v4 attached.
Regards,
Dean
From 5d1523cf58c58b90a8e4acc109fcbc32
n_date)
Is it really worth adding a core function for that?
Regards,
Dean
r a new thread, if there's demand.
Regards,
Dean
On Tue, 8 Jul 2025 at 09:51, Dean Rasheed wrote:
>
> Answering my own question, INSERT ... ON CONFLICT DO UPDATE does have
> the same problem as MERGE. To reproduce the error, all you need to do
> is create the unique index it needs *after* creating the publication
The other questio
On Thu, 10 Jul 2025 at 15:06, Dean Rasheed wrote:
>
> > Yes, perhaps we should convert src/tools/testint128.c into a new test
> > extension, src/test/modules/test_int128
>
> Here's an update doing that (in 0001). 0002-0005 are unchanged.
v3 attached, fixing a couple
On Wed, 9 Jul 2025 at 22:31, Dean Rasheed wrote:
>
> On Wed, 9 Jul 2025 at 18:27, Andres Freund wrote:
> >
> > I think we should wire this up to the buildsystem and our testsuite...
> > Having
> > testcode that is not run automatically may be helpful while origi
ility issues
> or regressions.
Yes, perhaps we should convert src/tools/testint128.c into a new test
extension, src/test/modules/test_int128
Regards,
Dean
On Wed, 9 Jul 2025 at 07:41, John Naylor wrote:
>
> Hi Dean, I went to take a look at this and got stuck at building the
> test file. The usual pointing gcc to the src and build include
> directories didn't cut it. How did you get it to work?
>
Yes, it wasn't immediatel
ault-deny policy
* will be applied instead, see get_row_security_policies.
*/
Regards,
Dean
On Mon, 7 Jul 2025 at 18:17, Dean Rasheed wrote:
>
> This makes me wonder though, does INSERT ... ON CONFLICT DO UPDATE
> have the same problem as MERGE?
>
Answering my own question, INSERT ... ON CONFLICT DO UPDATE does have
the same problem as MERGE. To reproduce the error, all yo
d
in excluded values, since that may have contributed to the row
being excluded from insertion.
Regards,
Dean
O foo VALUES (1)
ON CONFLICT (a) DO UPDATE SET b = 'yyy';
This isn't rejected, and it replicates the change to the subscriber
correctly, which makes me wonder, why do we need this restriction on
filter columns? If there is some other situation where it's needed,
should INSERT ... ON CONFLICT DO UPDATE also check it?
Regards,
Dean
user-defined function in SQL. Maybe
that's OK, if it's something that there's a lot of demand for, but
it's worth asking that question.
Regards,
Dean
ke one seems to be going in the
wrong direction. Perhaps it would help to re-examine the use cases,
and whether using the cube extension is the right thing in the first
place.
Regards,
Dean
ero-volume cubes (points), so translation-addition would work the
same as normal vector addition in that case. However, I doubt that
cube is ever going to make a good general-purpose type for vectors. It
would be somewhat odd to allow dot and cross products of cubes, for
example.
Regards,
Dean
gued that this is a bug fix, but the lack of prior
complaints justifies not back-patching.
Regards,
Dean
and database \"%s\".".
It's not clear what the "historical reasons" are for listTables()
being an exception, but if we're changing any of these, I'd say we
should make them all consistent.
Regards,
Dean
On Tue, 24 Jun 2025 at 19:49, Dean Rasheed wrote:
>
> The attached patch allows EXCLUDED values to appear in the RETURNING
> list of INSERT ... ON CONFLICT DO UPDATE.
>
> I still have a lot more testing to do, and docs to update, but so far
> the results look promising. I'
On Thu, 26 Jun 2025 at 04:04, Robert Treat wrote:
>
> At first look this seems right, modulo some typos
Oops, yes that was careless. Thanks for checking. I've fixed those and
pushed it.
Regards,
Dean
On Wed, 25 Jun 2025 at 15:53, Nathan Bossart wrote:
>
> Seems reasonable to me.
>
Pushed. Thanks for looking.
Regards,
Dean
Looking at the docs for MERGE, I realised that the "Synopsis" is not
indented consistently with other pages -- the convention on almost
every other page seems to be to indent continuation lines and options
by 4 spaces. So I think we should do the same for MERGE, as in the
attached.
Reg
On Thu, 16 Jan 2025 at 15:28, Dean Rasheed wrote:
>
> I went over this again in detail and didn't find any problems, so I
> have committed it. Thanks for all the review comments.
>
Looking at the doc pages for UPDATE and MERGE, I realise that I missed
a paragraph in the &quo
or RLS. I
think those updates might make it easier to test and document the RLS
aspects of this patch.
[1]
https://www.postgresql.org/message-id/flat/CAEZATCWqnfeChjK=n1v_dyzt4rt4mnq+ybf9c0qxdytvmsy...@mail.gmail.com
Regards,
Dean
support updatable
views and virtual generated columns turns out to be simpler if these
Vars have a separate varreturningtype.
I still have a lot more testing to do, and docs to update, but so far
the results look promising. I'll add this to the next CF.
Regards,
Dean
From fe79b310c287bf4bc6b3
code, 0005 gives a slight net reduction in the
total line count, and eliminates nearly all "#ifdef HAVE_INT128"
conditional code from numeric.c, making it significantly simpler and
easier to follow.
Testing on a 32-bit system without native int128 support, I see
something like a 1.3-1.5x spe
/*
> * Can only insert DEFAULT into generated columns, regardless of
> * any OVERRIDING clauses.
> */
> as a non-native English speaker, i interpret this as it implies that
> generated columns
> can have OVERRIDING clauses in some scenarios.
OK, fair enough. The suggested update looks reasonable to me.
Regards,
Dean
t.
Perhaps there's an alternate wording that would make that meaning more
explicit, rather than just removing that part of the comment, which
might leave people wondering why that code block isn't checking
"override", when the preceding code blocks do.
Regards,
Dean
On Wed, 28 May 2025 at 11:37, Tender Wang wrote:
>
> Dean Rasheed 于2025年5月28日周三 18:26写道：
>>
>> Unless there are any further comments, I plan to commit it in a day or so.
>
> I don't have any other comments. It looks good for me.
>
Thanks for looking. I have committed this now.
Regards,
Dean
being
correctly initialised for MERGE, which is what this patch aims to fix.
Unless there are any further comments, I plan to commit it in a day or
so.
Regards,
Dean
th mentioning 9428c001f67, which sped up numeric
division. That can be included in the same item, as in the attached
patch, unless you think it's worth listing it as a separate item.
Regards,
Dean
diff --git a/doc/src/sgml/release-18.sgml b/doc/src/sgml/release-18.sgml
new file mode 100
point to the same Relation. In
that case, we do still need to set up the WCO list, RETURNING list and
projection for rootRelInfo, but we don't need to map attribute
numbers. Building the attribute map looks like it's O(n^2) in the
number of attributes, so it's worth avoiding if we can.
Regards,
Dean
On Mon, 26 May 2025 at 07:46, Tender Wang wrote:
>
> Hi Dean,
>
> "it is possible for the parent to be excluded from the
> plan and so all of the entries in the resultRelInfo array may be for
> different relations than rootResultRelInfo."
>
> I didn't fu
ingList, but I don't think it really makes sense
to do it there. The patch intentionally only does it for a MERGE into
an inherited table when there are INSERT actions, and this also allows
the new code to be more consistent with ExecInitPartitionInfo().
Regards,
Dean
From 1d20f593cbc3f4546
On Sun, 25 May 2025 at 13:41, Dean Rasheed wrote:
>
> On Sun, 25 May 2025 at 13:06, Tender Wang wrote:
> >
> > For a partitioned table, we must pass rootResultRelInfo to ExecInsert(). I
> > added the check before calling ExecInsert()
> > If it is a partitio
o.
> Please see the attached diff file. The patch passed all regression test cases.
>
No, I don't think that's the right fix. I'm looking at it now, and I
think I have a fix, but it's more complicated than that. I'll post an
update later.
Regards,
Dean
ent. Do you have any
> proposal about the code?
>
Perhaps the way to do it is to make ChangeVarNodesExtended() take a
callback function to be invoked for each node visited. The callback
(which would then be in the planner, with the other SJE code) would do
any special handling it needed (for RangeTblRef and RestrictInfo
nodes), and call ChangeVarNodes_walker() for any other types of node,
to get the default behaviour.
Regards,
Dean
uot; also needs updating. There may be more places. I'd
suggest a bit of grepping in the docs (and probably also in the code)
for other places that need updating.
It also feels like this needs more regression tests, plus some new
isolation test cases.
Regards,
Dean
From ea50933617ec2fd42ffe192
coverage.
So patch 0001, attached, adds a new set of regression tests, near the
start of rowsecurity.sql, which specifically tests which policies are
applied for each command variant.
Patch 0002 updates the doc table to try to be clearer and more
accurate, and consistent with the test results f
can also indicate a pole error (e.g.,
> input 0 or a negative integer), not just overflow.
OK, thanks. I've updated that comment and committed it.
Regards,
Dean
DRF is to allow per-subscriber variations in change broadcasts,
enabling granular control over what data is sent to each subscriber based
on their session context.
Best,
Dean S
On Mon, Mar 17, 2025 at 4:32 AM Amit Kapila wrote:
> On Sun, Mar 16, 2025 at 12:59 AM Dean wrote:
> &g
replica filtering allows for session-specific, per-row, and
per-column filtering - features currently not supported by existing
replication filters, enhancing security and data privacy.
I look forward to hearing your thoughts!
Best,
Dean S
at way, the count always matches the number of rows returned
when there's a RETURNING clause, which I think is true of all other
DML commands.
Regards,
Dean
king along the same lines. This particular issue feels
a bit manufactured, and unlikely to occur in practice, but I'm happy
to go with whatever you decide.
Thanks for taking care of this.
Regards,
Dean
ss if that branch is
removed), but as a comment higher up says, that would be more
expensive. So perhaps this new comment should say "This is why it's
preferable to handle simple PHVs in the branch above, rather than this
branch."
Finally, ReplaceWrapOption should be added to pgindent's typedefs.list.
Regards,
Dean
On Sat, 1 Mar 2025 at 11:30, Dean Rasheed wrote:
>
> With those updates, I think this is ready for commit, which I'll do in
> a day or two, if there are no further comments.
>
Committed.
Regards,
Dean
he code shown
> above unnecessary, and we can simply remove it.
>
Yes, that makes sense, and it seems like a fairly straightforward
simplification, given that perform_pullup_replace_vars() sets it back
to false shortly afterwards.
The same thing happens in pull_up_constant_function().
Regards,
Dean
On Thu, 14 Nov 2024 at 22:35, Dean Rasheed wrote:
>
> On Thu, 14 Nov 2024 at 16:28, Dmitry Koval wrote:
> >
> > >SELECT gamma(float8 '1e-320');
> > ERROR: value out of range: overflow
> >
> > >SELECT gamma(float8 '0');
> >
aviour more
obvious.
* I also tweaked the regression tests a bit, and copied the existing
test style which displays both the expected and actual results from
each test.
With those updates, I think this is ready for commit, which I'll do in
a day or two, if there are no further comments.
Re
On Mon, 24 Feb 2025 at 09:21, Richard Guo wrote:
>
> Here are the updated patches with revised comments and some tweaks to
> the commit messages. I plan to push them in one or two days.
>
LGTM.
Regards,
Dean
from
a small runtime performance hit.
Given that we're moving this part of expanding virtual generated
columns to the planner, I wonder if we should also move the other bits
(build_generation_expression and expand_generated_columns_in_expr)
too, so that they're all together. That could be a follow-on patc
On Wed, 19 Feb 2025 at 01:42, Dean Rasheed wrote:
>
> One thing I don't like about this is that it's introducing more code
> duplication between pullup_replace_vars() and
> ReplaceVarsFromTargetList(). Those already had a lot of code in common
> before RETURNING OLD/N
VarsFromTargetList(). Those already had a lot of code in common
before RETURNING OLD/NEW was added, and this is duplicating even more
code. I think it'd be better to refactor so that they share common
code, since it has become quite complex, and it would be better to
have just one place
imitation to have. It would be
nice to have this fully working for the next release.
Attached is a rough patch that moves the expansion of virtual
generated columns to the planner. It needs a lot more testing (and
some regression tests), but it does seem to fix all the issues
mentioned in this thr
scribe output (and maybe also pg_dump) to explicitly output
"virtual" for columns of this kind. I know that that's the default for
generated columns, but someone reading the output might not know or
remember that, so perhaps it would be helpful to be explicit.
Regards,
Dean
d with CMD_INSERT.
I'm not sure if that represents an actual bug, but it looks wrong. It
should perhaps be called "ri_extraUpdatedCols_valid", and only set to
true when ExecInitGenerated() is called with CMD_UPDATE, and
ri_extraUpdatedCols is populated.
Regards,
Dean
nteger to a bytea
produces 2, 4, or 8 bytes, depending on the width of the integer type.
The result is the two's complement representation of the integer, with
the most significant byte first.", and then list the examples to
demonstrate that.
Regards,
Dean
be consistent with the idea that these are being regarded
primarily as bytea operations, rather than integer operations (just as
the bit <-> integer casts are documented in 9.6 "Bit String Functions
and Operators").
Regards,
Dean
nks. I hadn't tested qualified table names in the RETURNING list.
I've pushed a fix for that.
Regards,
Dean
On Wed, 8 Jan 2025 at 09:38, Dean Rasheed wrote:
>
> OK, done.
>
> I also noticed that I had failed to use quote_identifier() in
> ruleutils.c for the new WITH aliases, so I've fixed that and adjusted
> a couple of the test cases to test that.
>
I went over this again
t
in first, but the one that comes second will need to do something like
this.
Regards,
Dean
[1] https://commitfest.postgresql.org/51/4723/
virt-gen-cols-with-returning-old-new.patch.no-cfbot
Description: Binary data
On Thu, 9 Jan 2025 at 03:18, Greg Sabino Mullane wrote:
>
> On Wed, Jan 8, 2025 at 8:44 AM Dean Rasheed wrote:
>>
>> Attached is a more complete patch
>
> +1, looks good
>
Thanks for looking. I've pushed this now.
(I realised that I had missed \lo_list, so I a
ith a question mark,
because all other boolean columns in psql meta-commands end with a
question mark.
Regards,
Dean
xactly the same; the ending "?" matches
> things like "Key?" in \d of an index or the "Default?" column in \dc.
> It's a boolean, so people know how to read it.
>
By the time the filler words have been stripped out, the description
isn't really adding any useful information that can't be guessed from
the parameter name and value. So I tend to agree that it should be
left out (for now at least).
Regards,
Dean
y don't say what the range of
the type is. For example, we currently do:
select repeat('1', 50)::bit(50)::int;
ERROR: integer out of range
Regards,
Dean
"integer out of range" and "bigint out of
range" would be more consistent with existing error messages.
Regards,
Dean
ceVarsFromTargetList()) needs that when expanding whole-row
variables. Here's a simple reproducer which crashes:
CREATE TABLE foo (a int, b int GENERATED ALWAYS AS (a*2) VIRTUAL);
ALTER TABLE foo ADD CONSTRAINT foo_check CHECK (foo IS NOT NULL);
Regards,
Dean
do it that way round by default -- i.e., make it like
\dconfig and have 2 columns, "Parameter" and "Value", with lots of
rows.
Perhaps it could also have a "Description" column, which might help
with things like distinguishing between authenticated user and session
user.
Regards,
Dean
t and be fine.
>
Ah yes, I see. Thanks for that. It looks like the differences in
xml_2.out were for a specific change in libxml2 behaviour that only
affects error details in a small number of cases, so cloning the diffs
looks like it should be fine in this case.
Regards,
Dean
similar code that does quote variable names
properly).
I scanned the rest of ruleutils.c, and didn't find any other issues.
Regards,
Dean
From 868ea003a54adeeae470407321fe4481c95dfccb Mon Sep 17 00:00:00 2001
From: Dean Rasheed
Date: Sat, 11 Jan 2025 11:08:50 +
Subject: [PATCH v1 1/2
te_columns_pre_96[] needs to
be updated to support connecting to pre-9.6 servers.
The translate_columns entries for this new column should be true, so
that the "yes"/"no" gets appropriately translated. That means that
describeOperators() will need a similar translate_columns array.
Regards,
Dean
, '1234567890123456');
> \c - bar
> select * from t1; -- permission denied
> select id, ccredacted from t1; -- ok
Makes sense.
Regards,
Dean
in more detail. So in the end, I decided
to just add a sentence to each command's description, keeping it as
short as possible.
Regards,
Dean
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
new file mode 100644
index 72f3347..c48bcfb
--- a/doc/src/sgml/ref/psql-re
as had a decent amount of testing and review, so yes, I do plan to
commit it for v18.
In fact I think it is probably best to commit it soon, so that it has
more time in tree, ahead of v18. With that in mind, I plan to go over
it again in detail, and barring any other review comments, commit it.
Regards,
Dean
#x27;t looked at yet, but they should be easy to make work in the
same way, if people think this is useful.
Regards,
Dean
[1]
https://www.postgresql.org/message-id/flat/20240701220817.483f9b645b95611f8b1f6...@sranhm.sraoss.co.jp
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/ps
On Sat, 30 Nov 2024 at 00:38, tsinghualucky...@foxmail.com
wrote:
>
> Dear Dean Rasheed, I have reviewed the v4 patch and it is very thoughtful and
> reasonable, with a very clever attention to detail (plus I am very happy that
> we can get rid of the goto, which I was not
On Fri, 29 Nov 2024 at 13:10, Dean Rasheed wrote:
>
> There are a couple more things that I think need tidying up. I'll post an
> update when I get back to my computer.
>
Here's an update with some cosmetic tidying up, plus a couple of
not-so-cosmetic changes:
The new
cases. Here is the patch of version v3.
>
> It looks fine to me. The only things I'd adjust are stylistic,
> namely; 1) remove two tabs before the goto label, 2) remove redundant
> braces around the goto cleanup, 3) rename the variable "q" to
> something slightly more
amp;parsetree->hasRowSecurity);
rather than having a separate second step to update the flag on
"parsetree", and similarly for fireRIRrules()'s recursive calls to
itself. If, in the future, it becomes necessary to invoke
fireRIRrules() on more parts of a Query, it's then much more likely
that the new code won't forget to update the parent query's flag.
Regards,
Dean
be possible to write this using a
single additional allocated NumericVar and one init_var()/free_var()
pair.
There's no need to use floor(), since the div_var() call already
produces a floored integer result.
It could use some regression test cases.
Regards,
Dean
On Tue, 29 Oct 2024 at 13:05, Dean Rasheed wrote:
>
> Rebased version attached. No other changes.
>
In the light of 7f798aca1d5df290aafad41180baea0ae311b4ee, I guess I
should remove various (void *) casts that this patch adds, copied from
old code. I'll wait a while though, ju
ke a decision, and then standardise
> > on whatever people decide.
>
> I am not a native English speaker, but if this is natural spelling in
> English, I wonder we can replace all of them to leakproof without a hyphen.
>
Yes, I think we should do that (in a separate patch).
Regards,
Dean
the most useful thing to do
from a practical point of view, and if we do that for too-large
inputs, it makes sense to do the same for too-close-to-zero inputs.
OTOH, gamma(+/-0) = +/-Infinity is right from a mathematical point of
view, and consistent with the POSIX spec, and it's also consistent
with the functions cot() and cotd().
Regards,
Dean
RelationDesc,
PRS2_NEW_VARNO);
Regards,
Dean
I originally had in mind was doing it at the end of the RLS
policy loop, rather than adding another loop, as in the attached delta
patch. This passes all the current tests, and appears to work fine in
the new tests with RLS policies referring to virtual generated
columns.
Regards,
Dean
rls-fi
-bounds, like array_fill(). I.e., something like:
random_array(min int, max int, dims int[] [, lbounds int[]]) -> int[]
Returns an array filled with random values in the range min <= x <= max,
having dimensions of the lengths specified by dims. The optional lbounds
argument supplies lower-bound values for each dimension (which default
to all 1).
Regards,
Dean
tes random values, more
similar to the random(min, max) functions. Also I think it's more
useful if it shares the same PRNG, controlled by setseed(), and it
makes sense to group all such functions together.
Regards,
Dean
{{1,2},{3,4},{10,20}}
Regards,
Dean
ments so
far though, so I think we should make a decision, and then standardise
on whatever people decide.
Regards,
Dean
ter fixing that, I think it's sufficient to just list one
usage example.
Regards,
Dean
From 2a14be071dd2e721e768fdbaa57b096d509773aa Mon Sep 17 00:00:00 2001
From: Dean Rasheed
Date: Wed, 30 Oct 2024 08:41:41 +
Subject: [PATCH v3] tablefunc: Add rand_array() functions.
These functions
;t
look like those patches were committed, and there are still many such
warnings, if you try compiling postgres with those flags.
[1]
https://www.postgresql.org/message-id/flat/CA%2BhUKGJ7EQm9extQAgrFZNNUKqRT8Vv5t1tKqA-5nEcYn0%2BwNA%40mail.gmail.com
I don't know if anyone has any plans to pick up that work again, but
in any case, it seems wise to not add more.
Regards,
Dean
https://www.postgresql.org/docs/current/tablefunc.html
They really should have their own subsection, along the same lines as
"F.41.1.1. Normal_rand", explaining what the functions do, what their
arguments mean, and giving a couple of usage examples.
Regards,
Dean
ersed. For example,
imagine you wanted to use raw_expression_tree_walker() to print out
the whole structure of a raw parse tree. You'd want your printing
callback to be called for every node, including the ReturningOption
nodes.
Regards,
Dean
1 - 100 of 689 matches
Mail list logo
