On Sat, Nov 16, 2024 at 09:43:12PM +0100, Alvaro Herrera wrote:
> I just noticed that this commit broke pgaudit pretty thoroughly. I'm
> not sure if this means pgaudit needs changes, or this commit needs to be
> reconsidered in some way; at this point I'm just raising the alarm.
>
> (FWIW there a
On 2024-Oct-24, Michael Paquier wrote:
> Track more precisely query locations for nested statements
I just noticed that this commit broke pgaudit pretty thoroughly. I'm
not sure if this means pgaudit needs changes, or this commit needs to be
reconsidered in some way; at this point I'm just raisi
On Thu, Oct 24, 2024 at 11:28:55AM +0900, Michael Paquier wrote:
> Attached is the remaining piece, for DECLARE and CTAS. The
> JumbleQuery() calls in ExecCreateTableAs() and ExplainOneUtility() for
> CTAS queries are twins, showing the inner queries of CTAS
> consistently. DECLARE is covered by
On Wed, Oct 23, 2024 at 11:24:11AM +0200, Anthonin Bonnefoy wrote:
> This also answers another issue I was wondering about. Should the
> child's parsestate inherit the location information when
> make_parsestate is called? That would be incorrect since this is used
> for sub-statement, pstate shoul
On Wed, Oct 23, 2024 at 04:36:42PM +0800, jian he wrote:
> I am not sure of the meaning of "@$", though.
Please feel free to look at the upstream docs about that:
https://www.gnu.org/software/bison/manual/bison.html#Locations
"the location of the whole grouping is @$".
--
Michael
signature.asc
D
On Wed, Oct 23, 2024 at 8:10 AM Michael Paquier wrote:
>
> I have some more minor comments.
>
> -if (@$ < 0) /* see comments for YYLLOC_DEFAULT */
> -@$ = @2;
>
> With 14e5680eee19 now in the tree (interesting timing as this did not
> exist until yesterday), it looks li
On Wed, Oct 23, 2024 at 2:10 PM Michael Paquier wrote:
>
> On Tue, Oct 22, 2024 at 11:34:55AM +0200, Anthonin Bonnefoy wrote:
> > On Tue, Oct 22, 2024 at 7:06 AM Michael Paquier wrote:
> >> Something that
> >> worries me a bit is that this changes makes the code less clean, by
> >> having a SELEC
On Tue, Oct 22, 2024 at 11:34:55AM +0200, Anthonin Bonnefoy wrote:
> On Tue, Oct 22, 2024 at 7:06 AM Michael Paquier wrote:
>> Something that
>> worries me a bit is that this changes makes the code less clean, by
>> having a SELECT-INTO specific routine called in the parse-analyze
>> paths, while
On Tue, Oct 22, 2024 at 7:06 AM Michael Paquier wrote:
> -Query *
> -transformTopLevelStmt(ParseState *pstate, RawStmt *parseTree)
>
> What's the advantage of removing this routine? Is that because you're
> pushing the initialization of stmt_location and stmt_len into
> transformOptionalSelectInt
On Tue, Oct 22, 2024 at 02:06:16PM +0900, Michael Paquier wrote:
> I've looked at 0001, and finished by splitting the case of all-level
> tracking with the multi-statements as the resulting table was feeling
> heavily bloated, particularly because of MERGE that spawned in
> multiple lines even if t
On Mon, Oct 21, 2024 at 10:35:17AM +0200, Anthonin Bonnefoy wrote:
> I've updated 0001 to only use ORDER BY query. The query strings are
> not exact doublons, as the nested statement has the additional ending
> ';' due to using the whole string instead of just the RawStmt. Thus,
> the other sort ex
On Tue, Oct 15, 2024 at 10:11:40AM +0200, Anthonin Bonnefoy wrote:
> I've updated the patchset with additional tests for COPY in 0001. 0002
> includes the necessary modifications to handle COPY.
Beginning with the beginning of this patch series.
+SELECT toplevel, calls, query FROM pg_stat_stateme
On Thu, Oct 17, 2024 at 09:21:11AM +0200, Anthonin Bonnefoy wrote:
> I'm not sure about this approach. It moves the responsibility of
> tracking the location and length from the nested statement to the top
> level statement.
> - The location you added in ExplainStmt and CopyStmt has a different
> m
Hi Jian,
On Thu, Oct 17, 2024 at 5:59 AM jian he wrote:
> in [3] I mentioned adding "ParseLoc location" to ExplainStmt, then you
> found some problems at [4] with multi statements,
> now I found a way to resolve it.
> I also add "ParseLoc location;" to typedef struct CopyStmt.
> copy (select 1) t
hi. Anthonin
please check attached v9-0001, v9-0002, v9-003.
v9-0001-Better-error-reporting-from-extension-scripts-Was.patch
same as v4-0001-Improve-parser-s-reporting-of-statement-start-loc.patch in [1]
v9-0002-Add-tests-covering-pgss-nested-queries.patch same as
v8-0001-Add-tests-covering-pgss-
On Tue, Oct 15, 2024 at 3:23 PM jian he wrote:
> explain(verbose) SELECT 1, 2, 3\; explain SELECT 1, 2, 3, 4;
> will transformed to
> explain(verbose) SELECT 1, 2, 3; explain SELECT 1, 2, 3, 4;
>
> it seems to me your patch care about following position.
> explain(verbose
hi.
explain(verbose) SELECT 1, 2, 3\; explain SELECT 1, 2, 3, 4;
will transformed to
explain(verbose) SELECT 1, 2, 3; explain SELECT 1, 2, 3, 4;
it seems to me your patch care about following position.
explain(verbose) SELECT 1, 2, 3; explain SELECT 1, 2, 3,
On Wed, Oct 9, 2024 at 4:49 PM Anthonin Bonnefoy
wrote:
>
> Here is a new version of the patchset.
>
> 0001: Add tests to cover the current behaviour: Missing nested
> statements for CreateTableAs, DeclareCursor and MaterializedViews,
> nested statements reported by explain including the whole str
On Mon, Oct 7, 2024 at 7:39 AM Michael Paquier wrote:
> GOod point, this is confusing. The point is that having only
> stmt_location is not enough to detect where the element in the query
> you want to track is because it only points at its start location in
> the full query string. In an ideal
On Mon, Oct 7, 2024 at 1:39 PM Michael Paquier wrote:
>
> On Fri, Oct 04, 2024 at 08:16:00PM +0800, jian he wrote:
> > about v5 0001
> > select_with_parens:
> > '(' select_no_parens ')'{ $$ = $2; }
> > | '(' select_with_parens ')'{ $$ = $2; }
> >
On Fri, Oct 04, 2024 at 08:16:00PM +0800, jian he wrote:
> about v5 0001
> select_with_parens:
> '(' select_no_parens ')'{ $$ = $2; }
> | '(' select_with_parens ')'{ $$ = $2; }
> ;
>
>
> toplevel | calls | query
On Fri, Oct 4, 2024 at 5:05 PM Anthonin Bonnefoy
wrote:
>
> I agree that tracking 2 identical statements with different queryIds
> and nesting levels is very confusing. On the other hand, from an
> extension developer point of view (not necessarily limited to
> pg_stat_statements), I would like to
On Wed, Oct 2, 2024 at 6:39 AM Michael Paquier wrote:
> FWIW, I've always found this case with EXPLAIN with two entries
> confusing, so what's the advantage in trying to apply this rule for
> the rest? We know that EXPLAIN, DECLARE and CTAS run a query attached
> to their DDL, hence isn't it suff
On Fri, Aug 30, 2024 at 09:37:03AM +0200, Anthonin Bonnefoy wrote:
> Thanks for the review. I think the parser state is mostly used for the
> error callbacks and parser_errposition but I'm not 100% sure. Either
> way, you're right and it probably shouldn't be in the patch. I've
> modified the patch
On Mon, Sep 30, 2024 at 5:14 PM jian he wrote:
> Is it possible to normalize top level utilities explain query, make
> these two have the same queryid?
> explain(verbose) EXECUTE test_prepare_pgss1(1, 2);
> explain(verbose) EXECUTE test_prepare_pgss1(1, 3);
>
> I guess this is a corner case.
This
On Tue, Oct 01, 2024 at 09:26:40AM +0200, Anthonin Bonnefoy wrote:
> This seems to be a known issue. The test_prepare_pgss1's parameters
> are A_Const nodes. Those nodes have a custom query jumble which
> doesn't record location[1] and thus can't be normalised by pgss.
>
> That could be fixed by re
PREPARE test_prepare_pgss1(int, int) AS select generate_series($1, $2);
SELECT pg_stat_statements_reset() IS NOT NULL AS t;
EXECUTE test_prepare_pgss1 (1,2);
EXECUTE test_prepare_pgss1 (1,3);
SELECT toplevel, calls, query, queryid, rows FROM pg_stat_statements
ORDER BY query COLLATE "C", toplevel;
On Tue, Aug 27, 2024 at 11:14 AM jian he wrote:
> also it's ok to use passed (ParseState *pstate) for
> {
> estate = CreateExecutorState();
> estate->es_param_list_info = params;
> paramLI = EvaluateParams(pstate, entry, execstmt->params, estate);
> }
> ?
> I really don't k
On Mon, Aug 26, 2024 at 4:55 PM Anthonin Bonnefoy
wrote:
>
/* Evaluate parameters, if any */
if (entry->plansource->num_params)
{
- ParseState *pstate;
-
- pstate = make_parsestate(NULL);
- pstate->p_sourcetext = queryString;
you deleted the above these lines, but passed (ParseState *pstat
On Mon, Aug 26, 2024 at 5:26 AM jian he wrote:
>queryid| left
> | plans | calls | rows
> --+--+---+---+--
> 2800308901962295548 | EXPLAIN (verbose
On Mon, Aug 5, 2024 at 3:19 PM Anthonin Bonnefoy
wrote:
>
> I've realised my initial approach was wrong, calling the post parse
> for all nested queries in analyze.c prevents extension like pgss to
> correctly track the query's nesting level.
>
> I've changed the approach to replicate what's done
I've realised my initial approach was wrong, calling the post parse
for all nested queries in analyze.c prevents extension like pgss to
correctly track the query's nesting level.
I've changed the approach to replicate what's done in ExplainQuery to
both CreateTableAs and DeclareCursor: Jumble the
Hi all,
Some utility statements like Explain, CreateTableAs and DeclareCursor
contain a query which will be planned and executed. During post parse,
only the top utility statement is jumbled, leaving the contained query
without a query_id set. Explain does the query jumble in ExplainQuery
but for
33 matches
Mail list logo
