> On Tue, 22 Sep 2020 at 17:00, Hou, Zhijie wrote:
> > I found some more places that should use appendPQExrBufferStr instead
> of appendPQExpBuffer.
> >
> > Here is the patch.
>
> Seems like a good idea. Please add it to the next commitfest.
Thanks, added it
Hi
I made a slight update to the patch
> > > I found some more places that should use appendPQExrBufferStr instead
> > of appendPQExpBuffer.
> > >
> > > Here is the patch.
> >
> > Seems like a good idea. Please add it to the next commitfest.
>
> Thanks, added it to the next commitfest.
> https:
Hi,
In (src/test/regress/sql/aggregates.sql)
I found some tables is not dropped at the end of the sqlscript,
It does not hava any problem, but I think it's better to drop the table in time.
Please see the attachment for the patch.
Best regards,
Houzj
0001-enhance-test-for-hashdisk.patch
De
Hi
In (/src/backend/replication/backup_manifest.c)
I found the following code:
appendStringInfoString(&buf, "\n");
appendStringInfoString(&buf, "\"");
Since only one bit string is appended here,
I think it will be better to call appendStringInfoChar.
Best reagrds,
houzj
00
> Good point. There's another one:
> $ git grep -E 'appendStringInfoString.*".{,1}");'
> src/backend/utils/adt/ruleutils.c: appendStringInfoString(buf, "(");
> I noticed you added a similar thread here.
> https://commitfest.postgresql.org/30/
> I think this one could be combined as a singl
Hi
I found the function SPI_connect() has only one return value(SPI_OK_CONNECT).
But I also found that some places check the return value of SPI_connect() like
the following:
if (SPI_connect() != SPI_OK_CONNECT)
elog(ERROR, "SPI_connect failed");
Is it necessary to check
> > May be we can make "int SPI_connect()" => "void SPI_connect()" and fix
> the doc ?
>
> I don't see a lot of value in changing SPI_connect() in isolation.
> It's part of an API suite that generally expects functions to return error
> codes.
>
> There was a previous thread [1] about getting rid
Hi
In (/src/pl/plpgsql/src/pl_exec.c), I found some code like the following:
PG_CATCH();
{
if (expr->plan && !expr->plan->saved)
expr->plan = NULL;
PG_RE_THROW();
}
PG_END_TRY();
if (expr->plan && !ex
Hi
When looking into the code about hash disk,
I found some comment in nodeAgg.c may have not been updated.
1. Since function lookup_hash_entry() has been deleted,
there are still some comment talk about lookup_hash_entry().
* and is packed/unpacked in lookup_hash_entry() / agg_retrieve_hash_t
Hi
In multixact.c I found some comments like the following:
* Similar to AtEOX_MultiXact but for COMMIT PREPARED
* Discard the local MultiXactId cache like in AtEOX_MultiXact
Since there's no function called "AtEOX_MultiXact" in the code,
I think the "AtEOX_MultiXact" may be a typo
Hi
I have a look over this patch and find some typos in 0002.
1.Some typos about unique:
There are some spelling mistakes about "unique" in code comments and README.
Such as: "+However we define the UnqiueKey as below."
2.function name about initililze_uniquecontext_for_joinrel:
May be it should
Hi
I found some likely unnecessary if-condition in code.
1. Some check in else branch seems unnecessary.
In (/src/backend/replication/logical/reorderbuffer.c)
① @@ -4068,7 +4068,7 @@ ReorderBufferToastAppendChunk(ReorderBuffer *rb,
ReorderBufferTXN *txn,
> bool found;
> if (!found)
> {
>
Hi
I found some code places call list_delete_ptr can be replaced by
list_delete_xxxcell which can be faster.
diff --git a/src/backend/optimizer/path/joinpath.c
b/src/backend/optimizer/path/joinpath.c
index db54a6b..61ef7c8 100644
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/opt
> To me it looks good to clean up the conditions as you have done in the patch.
> Please add this to commitfest so that it's not forgotten. I have verified
> the code and indeed the conditions you are removing are unnecessary. So
> the patch can be marked as CFP right away.
Thank you for reviewing
> I found some code path use list_delete_ptr while the loop of foreach() is
> iterating.
>
> List_delete_ptr seems search the list again to find the target cell and
> delete it.
> > foreach(cell, list)
> > {
> > if (lfirst(cell) == datum)
> > return list_del
> > I found some code places call list_delete_ptr can be replaced by
> list_delete_xxxcell which can be faster.
> >
> > diff --git a/src/backend/optimizer/path/joinpath.c
> > b/src/backend/optimizer/path/joinpath.c
> > index db54a6b..61ef7c8 100644
> > --- a/src/backend/optimizer/path/joinpath.c
>
Hi
In /src/backend/executor/nodeAgg.c
I found the following comment still use work mem,
Since hash_mem has been introduced, Is it more accurate to use hash_mem here ?
@@ -1827,7 +1827,7 @@ hash_agg_set_limits(double hashentrysize, double
input_groups, int used_bits,
/*
* Don't
Hi Vignesh,
After having a look over the patch,
I have some suggestions for
0003-Allow-copy-from-command-to-process-data-from-file.patch.
1.
+static uint32
+EstimateCstateSize(ParallelContext *pcxt, CopyState cstate, List *attnamelist,
+ char **whereClauseStr, c
Hi
I found some code like the following:
> StringInfoData s;
> ...
> values[6] = CStringGetTextDatum(s.data);
The length of string can be found in ' StringInfoData.len',
but the macro CStringGetTextDatum will use strlen to count the length again.
I think we can use PointerGetDatum(cstring_to_te
Hi
I found the comment of function get_attgenerated(Oid relid, AttrNumber attnum)
seems wrong.
It seems the function is given the attribute number not the name.
/*
* get_attgenerated
*
- * Given the relation id and the attribute name,
+ * Given the relation id and the
On Tues, Sep 28, 2021 6:05 PM Amit Kapila wrote:
> On Tue, Sep 28, 2021 at 11:35 AM Masahiko Sawada
> wrote:
> >
> > On Tue, Sep 28, 2021 at 1:54 PM Amit Kapila
> wrote:
> > >
> > > >
> > > > Then, if, we proceed in this direction,
> > > > the place to implement those stats
> > > > would be on
101 - 121 of 121 matches
Mail list logo
