Re: remaining sql/json patches

2023-09-19 Thread Amit Langote
NULL. Maybe we can validate it in > transformJsonFuncExpr? > --- I'm not sure whether we should make the parser complain about the weird types being specified in RETURNING. The NULL you get in the above example is because of the following error: select json_qu

Re: remaining sql/json patches

2023-09-18 Thread Amit Langote
Hi Erik, On Mon, Sep 18, 2023 at 19:09 Erik Rijkers wrote: > Op 9/18/23 om 05:15 schreef Amit Langote: > > On Sun, Sep 17, 2023 at 3:34 PM Erik Rijkers wrote: > >> Op 9/14/23 om 10:14 schreef Amit Langote: > >>> > >>> > >> > >> Hi

Re: dubious warning: FORMAT JSON has no effect for json and jsonb types

2023-08-21 Thread Amit Langote
> > Anyone remember why this is here? Should we remove it? > > > > > > +1 for removing, on the basis that it is not suprising, and would > > pollute logs for most configurations. > > done +1 and thanks. May have been there as a debugging aid if anything. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: remaining sql/json patches

2023-08-15 Thread Amit Langote
t; line should be ON EMPTY ? Correct too. > Other than that, the doc looks good. Thanks for the review. I will post a new version after finishing working on a few other improvements I am working on. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: generic plans and "initial" pruning

2023-08-11 Thread Amit Langote
On Fri, Aug 11, 2023 at 14:31 Amit Langote wrote: > On Wed, Aug 9, 2023 at 1:05 AM Robert Haas wrote: > > On Tue, Aug 8, 2023 at 10:32 AM Amit Langote > wrote: > > > But should ExecInitNode() subroutines return the partially initialized > > > PlanState node or

Re: initial pruning in parallel append

2023-08-09 Thread Amit Langote
On Wed, Aug 9, 2023 at 9:48 PM Robert Haas wrote: > On Wed, Aug 9, 2023 at 6:22 AM Amit Langote wrote: > > > > I'm assuming it's not > > > > too ugly if ExecInitAppend() uses IsParallelWorker() to decide whether > > > > it should be writing to EStat

Re: initial pruning in parallel append

2023-08-09 Thread Amit Langote
On Tue, Aug 8, 2023 at 11:16 PM Robert Haas wrote: > On Tue, Aug 8, 2023 at 2:58 AM Amit Langote wrote: > > Or we could consider something like the patch I mentioned in my 1st > > email. The idea there was to pass the pruning result via a separate > > channel, not

Re: generic plans and "initial" pruning

2023-08-08 Thread Amit Langote
On Tue, Aug 8, 2023 at 12:36 AM Robert Haas wrote: > On Thu, Aug 3, 2023 at 4:37 AM Amit Langote wrote: > > Here's a patch set where the refactoring to move the ExecutorStart() > > calls to be closer to GetCachedPlan() (for the call sites that use a > > CachedPlan) is extr

Re: initial pruning in parallel append

2023-08-08 Thread Amit Langote
On Tue, Aug 8, 2023 at 12:53 AM Robert Haas wrote: > On Mon, Aug 7, 2023 at 10:25 AM Amit Langote wrote: > > Note we’re talking here about “initial” pruning that occurs during > > ExecInitNode(). Workers are only launched during ExecGather[Merge]() which > > there

Re: initial pruning in parallel append

2023-08-07 Thread Amit Langote
during ExecInitNode(). Workers are only launched during ExecGather[Merge]() which thereafter do ExecInitNode() on their copy of the the plan tree. So if we are to pass the pruning results for cross-checking, it will have to be from the leader to workers. > -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: remaining sql/json patches

2023-08-04 Thread Amit Langote
Hi, On Fri, Aug 4, 2023 at 19:01 Erik Rijkers wrote: > Op 7/21/23 om 12:33 schreef Amit Langote: > > > > Thanks for taking a look. > > > > Hi Amit, > > Is there any chance to rebase the outstanding SQL/JSON patches, (esp. > json_query)? Yes, wor

Re: remaining sql/json patches

2023-07-28 Thread Amit Langote
time do not have tuples in the > pg_proc catalog. Is it unnecessary? Yes. These are not functions that get pg_proc entries, but SQL constructs that *look like* functions, similar to XMLEXISTS(), etc. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: In Postgres 16 BETA, should the ParseNamespaceItem have the same index as it's RangeTableEntry?

2023-07-26 Thread Amit Langote
estate(pstate); > > return resultRelInfo; > } > > In this case, how can we get the relinfo->ri_RootResultRelInfo to store the > appropriate data? Your function doesn't seem to have access to the ModifyTableState node, so setting ri_RootResultRelInfo to the correct ResultRelInfo node does not seem doable. As I suggested in my previous reply, please check if passing 0 (not list_length(estate->es_range_table)) for the 3rd argument InitResultRelInfo() fixes the problem and gives the correct result. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: remaining sql/json patches

2023-07-26 Thread Amit Langote
On Fri, Jul 21, 2023 at 7:33 PM Amit Langote wrote: > On Fri, Jul 21, 2023 at 1:02 AM Alvaro Herrera > wrote: > > On 2023-Jul-21, Amit Langote wrote: > > > > > I’m thinking of pushing 0001 and 0002 tomorrow barring objections. > > > > 0001 looks reaso

Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns

2023-07-26 Thread Amit Langote
ate->opts.force_quote_all) { int i; for (i = 0; i < num_phys_attrs; i++) cstate->opts.force_quote_flags[i] = true; } Perhaps we could fix the inconsistency by changing the force_quote_all code to use MemSet() too. I'll defer whether to do that to Andrew's judgement. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: remaining sql/json patches

2023-07-20 Thread Amit Langote
On Thu, Jul 20, 2023 at 17:19 Amit Langote wrote: > On Wed, Jul 19, 2023 at 5:17 PM Amit Langote > wrote: > > On Wed, Jul 19, 2023 at 12:53 AM Alvaro Herrera > wrote: > > > On 2023-Jul-18, Amit Langote wrote: > > > > > > > Attached upd

Re: remaining sql/json patches

2023-07-20 Thread Amit Langote
Hello, On Thu, Jul 20, 2023 at 10:35 AM jian he wrote: > On Tue, Jul 18, 2023 at 5:11 PM Amit Langote wrote: > > > Op 7/17/23 om 07:00 schreef jian he: > > > > hi. > > > > seems there is no explanation about, json_api_common_syntax in > > > &g

Re: remaining sql/json patches

2023-07-19 Thread Amit Langote
On Wed, Jul 19, 2023 at 5:17 PM Amit Langote wrote: > On Wed, Jul 19, 2023 at 12:53 AM Alvaro Herrera > wrote: > > On 2023-Jul-18, Amit Langote wrote: > > > b6e1157e7d Don't include CaseTestExpr in JsonValueExpr.formatted_expr > > > > I feel a bit uneasy ab

Re: remaining sql/json patches

2023-07-19 Thread Amit Langote
On Wed, Jul 19, 2023 at 12:53 AM Alvaro Herrera wrote: > On 2023-Jul-18, Amit Langote wrote: > > > Attached updated patches. In 0002, I removed the mention of the > > RETURNING clause in the JSON(), JSON_SCALAR() documentation, which I > > had forgotten to do in the la

Re: In Postgres 16 BETA, should the ParseNamespaceItem have the same index as it's RangeTableEntry?

2023-07-18 Thread Amit Langote
ResultRelInfo pointing at the wrong RTE via its ri_RangeTableIndex. That code should perhaps set the ri_RangeTableIndex to 0 if it doesn't know the actual existing RTE corresponding to that result relation. If you set it to some non-0 value, the RTE that it points to should satisfy invariants such as having the corresponding RTEPermissionInfo present in the rteperminfos list if necessary. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: In Postgres 16 BETA, should the ParseNamespaceItem have the same index as it's RangeTableEntry?

2023-07-13 Thread Amit Langote
() most likely via markRTEForSelectPriv()) is not expecting to be called with? I would be helpful to see a backtrace when the error occurs to be sure. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: Eliminating SPI from RI triggers - take 2

2023-07-10 Thread Amit Langote
On Mon, Jul 10, 2023 at 5:27 PM Daniel Gustafsson wrote: > > On 21 Mar 2023, at 06:03, Amit Langote wrote: > > On Tue, Mar 21, 2023 at 3:54 AM Gregory Stark (as CFM) > > wrote: > >> On Mon, 17 Oct 2022 at 14:59, Robert Haas wrote: > > >>> But I t

Re: remaining sql/json patches

2023-07-07 Thread Amit Langote
On Fri, Jul 7, 2023 at 8:31 PM Peter Eisentraut wrote: > On 21.06.23 10:25, Amit Langote wrote: > > I realized that the patch for the "other sql/json functions" part is > > relatively straightforward and has no dependence on the "sql/json > > query function

Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)

2023-06-30 Thread Amit Langote
On Wed, Jun 28, 2023 at 4:30 PM Amit Langote wrote: > > Hi, > > On Tue, Feb 21, 2023 at 4:12 PM Amit Langote wrote: > > On Tue, Feb 21, 2023 at 12:40 AM Tom Lane wrote: > > > Alvaro Herrera writes: > > > > On 2023-Feb-20, Amit Langote wrote: > >

Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)

2023-06-28 Thread Amit Langote
Hi, On Tue, Feb 21, 2023 at 4:12 PM Amit Langote wrote: > On Tue, Feb 21, 2023 at 12:40 AM Tom Lane wrote: > > Alvaro Herrera writes: > > > On 2023-Feb-20, Amit Langote wrote: > > >> One more thing we could try is come up with a postgres_fdw tes

initial pruning in parallel append

2023-06-27 Thread Amit Langote
ssible, match_clause_to_partition_key() may pick one as a comparison function for pruning, because it doesn't actually check the procedure's provolatile before doing so. I'd hope not, though would like to be sure to support what I wrote above. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com [1]

Re: SQL/JSON revisited

2023-06-19 Thread Amit Langote
ommitfest.postgresql.org/43/4377/ -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: Fix a typo in rewriteHandler.c

2023-06-15 Thread Amit Langote
On Fri, Jun 16, 2023 at 10:25 AM Amit Langote wrote: > On Thu, Jun 15, 2023 at 5:07 PM Sho Kato (Fujitsu) > wrote: > > I've attached the patch for the following rewriteTargetView comments. > > > > s/rewriteQuery/RewriteQuery > > Good catch and thanks for the pa

Re: Fix a typo in rewriteHandler.c

2023-06-15 Thread Amit Langote
recurse through rewriteQuery, which will invoke > * rewriteTargetListIU again on the updated targetlist. > */ > if (parsetree->commandType != CMD_DELETE) > { > foreach(lc, parsetree->targetList) > > s/rewriteQuery/RewriteQuery Good catch and thanks for the patch. Will push shortly. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: obsolete filename reference in parser README

2023-06-15 Thread Amit Langote
On Thu, Jun 15, 2023 at 10:48 PM Tristan Partin wrote: > Nice catch. Looks good. Thanks for checking. As just mentioned, I've pushed this moments ago. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: obsolete filename reference in parser README

2023-06-15 Thread Amit Langote
On Thu, Jun 15, 2023 at 6:54 PM Amit Langote wrote: > I noticed that 2f2b18bd3f55 forgot to remove the mention of > parse_jsontable.c in src/backend/parser/README. > > Attached a patch to fix that. Will push that shortly to HEAD and v15. Pushed to HEAD only. 9853bf6ab0

obsolete filename reference in parser README

2023-06-15 Thread Amit Langote
Hi, I noticed that 2f2b18bd3f55 forgot to remove the mention of parse_jsontable.c in src/backend/parser/README. Attached a patch to fix that. Will push that shortly to HEAD and v15. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com v1-0001-Remove-outdated-reference-to-a-removed

Re: Views no longer in rangeTabls?

2023-06-14 Thread Amit Langote
On Wed, Jun 14, 2023 at 15:49 Amit Langote wrote: > On Wed, Jun 14, 2023 at 15:44 Michael Paquier wrote: > >> On Wed, Jun 14, 2023 at 02:34:56PM +0900, Amit Langote wrote: >> > This being my first commit, I intently looked to check if everything’s >> set >>

Re: Views no longer in rangeTabls?

2023-06-14 Thread Amit Langote
On Wed, Jun 14, 2023 at 15:44 Michael Paquier wrote: > On Wed, Jun 14, 2023 at 02:34:56PM +0900, Amit Langote wrote: > > This being my first commit, I intently looked to check if everything’s > set > > up correctly. While it seemed to have hit gitweb and GitHub, it didn’t &g

Re: Views no longer in rangeTabls?

2023-06-13 Thread Amit Langote
On Wed, Jun 14, 2023 at 12:08 Amit Langote wrote: > On Tue, Jun 13, 2023 at 9:40 PM David Steele wrote: > > On 6/13/23 11:38, Amit Langote wrote: > > > On Tue, Jun 13, 2023 at 6:33 PM Alvaro Herrera < > alvhe...@alvh.no-ip.org> wrote: > > >> Note that y

Re: Views no longer in rangeTabls?

2023-06-13 Thread Amit Langote
On Tue, Jun 13, 2023 at 9:40 PM David Steele wrote: > On 6/13/23 11:38, Amit Langote wrote: > > On Tue, Jun 13, 2023 at 6:33 PM Alvaro Herrera > > wrote: > >> Note that you changed one comment from "permission checks" to > >> "permission he

Re: Views no longer in rangeTabls?

2023-06-13 Thread Amit Langote
On Tue, Jun 13, 2023 at 6:33 PM Alvaro Herrera wrote: > Note that you changed one comment from "permission checks" to > "permission hecks". Oops, thanks for pointing that out. Fixed in the attached. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: Views no longer in rangeTabls?

2023-06-13 Thread Amit Langote
On Tue, Jun 13, 2023 at 4:44 PM David Steele wrote: > On 6/13/23 06:09, Amit Langote wrote: > > On Sat, Jun 10, 2023 at 10:27 PM Tom Lane wrote: > >> Julien Rouhaud writes: > >>> On Sat, Jun 10, 2023 at 08:56:47AM -0400, Tom Lane wrote: > >>>> -

Re: Views no longer in rangeTabls?

2023-06-12 Thread Amit Langote
p; OidIsValid(rte->relid)" condition, but that seemed like an overkill, so only added one in the #ifdef USE_ASSERT_CHECKING block in ExecCheckPermissions() that f75cec4fff877 added. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com From f7390a898b7e156d75372d28ea5698d2ced9795b Mon Sep 17

Re: Views no longer in rangeTabls?

2023-06-10 Thread Amit Langote
t; > > If you see "rte->rtekind == RTE_SUBQUERY && OidIsValid(rte->relid)", > > it's dead certain that relid refers to a view, so you could just wire > > in that knowledge. > > Yeah, that's a good trick. Even so, I wonder why relkind is not set when > rel

Re: Views no longer in rangeTabls?

2023-06-09 Thread Amit Langote
bout views. > -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: The documentation for READ COMMITTED may be incomplete or wrong

2023-05-19 Thread Amit Langote
the EPQ during LockRows? > What I'm thinking about doing to back-patch this is to replace > one of the pointer fields in EPQState with a pointer to a > subsidiary palloc'd structure, where we can put the new fields > along with the cannibalized old one. We've done something > simila

Re: "variable not found in subplan target list"

2023-05-04 Thread Amit Langote
it. (Wouldn’t have been able to get to it till Monday myself.) > -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: New committers: Nathan Bossart, Amit Langote, Masahiko Sawada

2023-04-21 Thread Amit Langote
On Fri, Apr 21, 2023 at 17:52 Masahiko Sawada wrote: > On Fri, Apr 21, 2023 at 2:40 AM Tom Lane wrote: > > > > The Core Team would like to extend our congratulations to > > Nathan Bossart, Amit Langote, and Masahiko Sawada, who have > > accepted invitations to

Re: on placeholder entries in view rule action query's range table

2023-04-05 Thread Amit Langote
On Thu, Apr 6, 2023 at 3:33 Tom Lane wrote: > Amit Langote writes: > > While thinking about query view locking in context of [1], I realized > > that we have missed also fixing AcquirePlannerLocks() / > > ScanQueryForLocks() to consider that an RTE_SUBQUERY rte may belong

Re: generic plans and "initial" pruning

2023-04-05 Thread Amit Langote
On Tue, Apr 4, 2023 at 10:29 PM Amit Langote wrote: > On Tue, Apr 4, 2023 at 6:41 AM Tom Lane wrote: > > A few concrete thoughts: > > > > * I understand that your plan now is to acquire locks on all the > > originally-named tables, then do permissions checks (which

Re: on placeholder entries in view rule action query's range table

2023-04-04 Thread Amit Langote
On Thu, Jan 12, 2023 at 10:06 AM Tom Lane wrote: > Amit Langote writes: > > On Mon, Jan 9, 2023 at 5:58 AM Tom Lane wrote: > >> Conceivably we could make it work by allowing RTE_SUBQUERY RTEs to > >> carry a relation OID and associated RTEPermissionInfo, so that when

Re: generic plans and "initial" pruning

2023-04-04 Thread Amit Langote
On Tue, Apr 4, 2023 at 6:41 AM Tom Lane wrote: > Amit Langote writes: > > [ v38 patchset ] > > I spent a little bit of time looking through this, and concluded that > it's not something I will be wanting to push into v16 at this stage. > The patch doesn't seem very close

Re: SQL/JSON revisited

2023-04-04 Thread Amit Langote
ed into one of the existing > tables. I didn't actually review the docs. I made the jsonfuncs.c changes to use soft error handling when needed, so I took a stab at that; attached a delta patch, which also fixes the problematic comments mentioned by Alexander in his comments 1 and 3. I'll need to spend some time to address other points. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com v15-0002-delta.patch Description: Binary data

Re: "variable not found in subplan target list"

2023-03-29 Thread Amit Langote
ep ec386948948 that introduced the notion of part_prune_index around if the project that needed it [1] has moved on to an entirely different approach altogether, one that doesn't require hacking up the pruning code. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com [1] https://commitfest.postgresql

Re: SQL/JSON revisited

2023-03-27 Thread Amit Langote
d8); jsonb_object_agg_unique_strict {} (1 row) postgres=# SELECT jsonb_object_agg_unique_strict('1', null::xid8); jsonb_object_agg_unique_strict {} (1 row) SELECT jsonb_object_agg_unique_strict('1', '1'::xid8); jsonb_obje

Re: Eliminating SPI from RI triggers - take 2

2023-03-20 Thread Amit Langote
than in the next couple of weeks for this release. My apologies that I didn't withdraw the patch sooner. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: A bug with ExecCheckPermissions

2023-03-09 Thread Amit Langote
respondingly, as in the attached? Agree it looks cleaner and self-explanatory that way. Thanks. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: generic plans and "initial" pruning

2023-03-02 Thread Amit Langote
On Wed, Feb 8, 2023 at 7:31 PM Amit Langote wrote: > On Tue, Feb 7, 2023 at 23:38 Andres Freund wrote: >> The tests seem to frequently hang on freebsd: >> https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest%2F42%2F3478 > > Thanks for the heads up. I’ve

Re: SQL/JSON revisited

2023-02-27 Thread Amit Langote
On Mon, Feb 27, 2023 at 4:45 PM Amit Langote wrote: > On Tue, Feb 21, 2023 at 2:25 AM Andres Freund wrote: > > Evaluating N expressions for a json table isn't a good approach, both memory > > and CPU efficiency wise. > > Are you referring to JsonTableInitOpaque() initial

Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)

2023-02-20 Thread Amit Langote
On Tue, Feb 21, 2023 at 12:40 AM Tom Lane wrote: > Alvaro Herrera writes: > > On 2023-Feb-20, Amit Langote wrote: > >> One more thing we could try is come up with a postgres_fdw test case, > >> because it uses the RelOptInfo.userid value for remote-costs-base

Re: SQL/JSON revisited

2023-02-20 Thread Amit Langote
On Tue, Feb 21, 2023 at 12:09 PM Amit Langote wrote: > On Mon, Feb 20, 2023 at 11:41 PM Erik Rijkers wrote: > > Op 20-02-2023 om 08:35 schreef Amit Langote: > > > Rebased again over queryjumble overhaul. > > But the following statement is a problem. It does not crash b

Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)

2023-02-19 Thread Amit Langote
On Fri, Feb 17, 2023 at 9:02 PM Alvaro Herrera wrote: > On 2022-Dec-11, Amit Langote wrote: > > While staring at the build_simple_rel() bit mentioned above, I > > realized that this code fails to set userid correctly in the > > inheritance parent rels that are child relatio

Re: ExecRTCheckPerms() and many prunable partitions (sqlsmith)

2023-02-13 Thread Amit Langote
On Mon, Feb 13, 2023 at 22:31 Tom Lane wrote: > Amit Langote writes: > > On Mon, Feb 13, 2023 at 5:07 Justin Pryzby wrote: > >> That seems to add various elog()s which are hit frequently by sqlsmith: > > > Thanks for the report. I’ll take a look once I’m back at a

Re: ExecRTCheckPerms() and many prunable partitions (sqlsmith)

2023-02-13 Thread Amit Langote
On Mon, Feb 13, 2023 at 5:07 Justin Pryzby wrote: > On Tue, Nov 29, 2022 at 10:37:56PM +0900, Amit Langote wrote: > > 0002 contains changes that has to do with changing how we access > > checkAsUser in some foreign table planning/execution code sites. > > Thought it might

Re: A bug with ExecCheckPermissions

2023-02-09 Thread Amit Langote
Hi, On Thu, Feb 9, 2023 at 14:44 Sergey Shinderuk wrote: > On 08.02.2023 21:23, Alvaro Herrera wrote: > > On 2023-Feb-08, Amit Langote wrote: > > > >> On Wed, Feb 8, 2023 at 16:19 Alvaro Herrera > wrote: > > > >>> I think we should a

Re: A bug with ExecCheckPermissions

2023-02-08 Thread Amit Langote
RTEs, for example. Also, it doesn’t make much sense to reinstate the original loop over range table and fetch the RTEPermissionInfo for the RTEs with non-0 perminfoindex, because the main goal of the patch was to make ExecCheckPermissions() independent of range table length. > -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: generic plans and "initial" pruning

2023-02-08 Thread Amit Langote
On Tue, Feb 7, 2023 at 23:38 Andres Freund wrote: > Hi, > > On 2023-02-03 22:01:09 +0900, Amit Langote wrote: > > I've added a test case under src/modules/delay_execution by adding a > > new ExecutorStart_hook that works similarly as > > delay_execution_planner().

Re: generic plans and "initial" pruning

2023-02-03 Thread Amit Langote
On Thu, Feb 2, 2023 at 11:49 PM Amit Langote wrote: > On Fri, Jan 27, 2023 at 4:01 PM Amit Langote wrote: > > I didn't actually go with calling the plancache on every lock taken on > > a relation, that is, in ExecGetRangeTableRelation(). One thing about > > doing it that wa

Re: generic plans and "initial" pruning

2023-02-02 Thread Amit Langote
On Fri, Jan 27, 2023 at 4:01 PM Amit Langote wrote: > On Fri, Jan 20, 2023 at 12:52 PM Amit Langote wrote: > > Alright, I'll try to get something out early next week. Thanks for > > all the pointers. > > Sorry for the delay. Attached is what I've come up with so far. >

Re: Speed up transaction completion faster after many relations are accessed in a transaction

2023-01-31 Thread Amit Langote
ller knows what those locks are, it can pass them as an array. * That speeds up the call significantly, when a lot of locks are held * (e.g pg_dump with a large schema). Otherwise, pass NULL for locallocks, * and we'll traverse through our hash table to find them. */ -- Thanks, A

Re: wrong Append/MergeAppend elision?

2023-01-26 Thread Amit Langote
On Fri, Jan 27, 2023 at 5:43 AM Tom Lane wrote: > David Rowley writes: > > On Fri, 27 Jan 2023 at 01:30, Amit Langote wrote: > >> It seems that the planner currently elides an Append/MergeAppend that > >> has run-time pruning info (part_prune_index) set, but

wrong Append/MergeAppend elision?

2023-01-26 Thread Amit Langote
-time pruning doesn't kick in to prune p1, even though PartitionPruneInfo to do so has been generated. Attached find a patch to fix that. There are some expected output diffs in partition_prune suite, though they all look sane to me. Thoughts? -- Thanks, Amit Langote EDB: http

Re: generic plans and "initial" pruning

2023-01-19 Thread Amit Langote
On Fri, Jan 20, 2023 at 12:58 PM Tom Lane wrote: > Amit Langote writes: > > On Fri, Jan 20, 2023 at 12:31 PM Tom Lane wrote: > >> It might be possible to incorporate this pointer into PlannedStmt > >> instead of passing it separately. > > > Yeah, that wo

Re: generic plans and "initial" pruning

2023-01-19 Thread Amit Langote
On Fri, Jan 20, 2023 at 12:31 PM Tom Lane wrote: > Amit Langote writes: > > On Fri, Jan 20, 2023 at 4:39 AM Tom Lane wrote: > >> I had what felt like an epiphany: the whole problem arises because the > >> system is wrongly factored. We should get rid of AcquireEx

Re: generic plans and "initial" pruning

2023-01-19 Thread Amit Langote
by locking, before doing anything else. We would have initialized the QueryDesc and the EState, but only minimally. That also keeps the PartitionPruneResult business local to the executor. Would you like me to hack up a PoC or are you already on that? -- Thanks, Amit Langote EDB: http://www.enterprisedb.com [1] https://www.postgresql.org/message-id/CA%2BHiwqG7ZruBmmih3wPsBZ4s0H2EhywrnXEduckY5Hr3fWzPWA%40mail.gmail.com

Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)

2023-01-19 Thread Amit Langote
On Tue, Jan 17, 2023 at 7:33 PM Alvaro Herrera wrote: > On 2022-Dec-12, Amit Langote wrote: > > On Sun, Dec 11, 2022 at 6:25 PM Amit Langote > > wrote: > > > I've attached 0001 to remove those extraneous code blocks and add a > > > comment mentioning t

Re: on placeholder entries in view rule action query's range table

2023-01-11 Thread Amit Langote
On Thu, Jan 12, 2023 at 12:45 PM Tom Lane wrote: > Amit Langote writes: > > On Thu, Jan 12, 2023 at 10:06 AM Tom Lane wrote: > >> I've pushed this with some cleanup --- aside from fixing > >> outfuncs/readfuncs, I did some more work on the comments, which > >&g

Re: ATTACH PARTITION seems to ignore column generation status

2023-01-11 Thread Amit Langote
On Thu, Jan 12, 2023 at 5:58 AM Tom Lane wrote: > Amit Langote writes: > > I've updated your disallow-generated-child-columns-2.patch to do this, > > and have also merged the delta post that I had attached with my last > > email, whose contents you sound to agree with.

Re: on placeholder entries in view rule action query's range table

2023-01-11 Thread Amit Langote
On Thu, Jan 12, 2023 at 10:06 AM Tom Lane wrote: > Amit Langote writes: > > On Mon, Jan 9, 2023 at 5:58 AM Tom Lane wrote: > >> Conceivably we could make it work by allowing RTE_SUBQUERY RTEs to > >> carry a relation OID and associated RTEPermissionInfo, so that when

Re: ATTACH PARTITION seems to ignore column generation status

2023-01-10 Thread Amit Langote
On Wed, Jan 11, 2023 at 7:13 AM Tom Lane wrote: > I wrote: > > Amit Langote writes: > >> Thanks for the patch. It looks good, though I guess you said that we > >> should also change the error message that CREATE TABLE ... PARTITION > >> OF emits to match the

Re: ATTACH PARTITION seems to ignore column generation status

2023-01-09 Thread Amit Langote
we'd have heard bug reports. Thanks for the patch. It looks good, though I guess you said that we should also change the error message that CREATE TABLE ... PARTITION OF emits to match the other cases while we're here. I've attached a delta patch. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com partition-generated-cols-different-error.patch Description: Binary data

Re: ATTACH PARTITION seems to ignore column generation status

2023-01-06 Thread Amit Langote
would have been locked. Patch doing it that way is attached. Perhaps the newly added error message should match CREATE TABLE .. PARTITION OF's, but I found the latter to be not detailed enough, or maybe that's just me. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com disallow-partition-only-generated-cols.patch Description: Binary data

Re: moving extraUpdatedCols out of RangeTblEntry (into ModifyTable)

2023-01-05 Thread Amit Langote
On Fri, Jan 6, 2023 at 3:33 PM Tom Lane wrote: > Amit Langote writes: > > BTW, you wrote in the commit message: > > (At present it seems that we don't enforce that for partitioning > > either, which is likely wrong to some degree or other; but the case > >

Re: moving extraUpdatedCols out of RangeTblEntry (into ModifyTable)

2023-01-05 Thread Amit Langote
On Fri, Jan 6, 2023 at 12:28 AM Tom Lane wrote: > Amit Langote writes: > > On Thu, Jan 5, 2023 at 4:59 AM Tom Lane wrote: > >> I've not looked into what it'd take to back-patch this. We can't > >> add a field to ResultRelInfo in released branches (cf 4b3e37993)

Re: moving extraUpdatedCols out of RangeTblEntry (into ModifyTable)

2023-01-05 Thread Amit Langote
behaviors in the same area. > > I've not looked into what it'd take to back-patch this. We can't > add a field to ResultRelInfo in released branches (cf 4b3e37993), > but we might be able to repurpose RangeTblEntry.extraUpdatedCols. I think we can make that work. Would you like me to give that a try? -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: SQL/JSON revisited

2022-12-27 Thread Amit Langote
On Wed, Dec 28, 2022 at 4:28 PM Amit Langote wrote: > > Hi, > > Rebased the SQL/JSON patches over the latest HEAD. I've decided to > keep the same division of code into individual commits as that > mentioned in the revert commit 2f2b18bd3f, squashing fixup comm

Re: Avoid lost result of recursion (src/backend/optimizer/util/inherit.c)

2022-12-26 Thread Amit Langote
plus that test. > > It feels a bit like famine to feast when it comes to tests for this bug today. Thanks for working on this. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: Avoid lost result of recursion (src/backend/optimizer/util/inherit.c)

2022-12-22 Thread Amit Langote
On Fri, Dec 23, 2022 at 1:04 PM Richard Guo wrote: > On Fri, Dec 23, 2022 at 11:22 AM Amit Langote wrote: >> Attached shows a test case I was able to come up with that I can see >> is broken by a61b1f74 though passes after applying Richard's patch. >> What's broken is

Re: Avoid lost result of recursion (src/backend/optimizer/util/inherit.c)

2022-12-22 Thread Amit Langote
On Fri, Dec 23, 2022 at 12:22 PM Amit Langote wrote: > Attached shows a test case I was able to come up with that I can see > is broken by a61b1f74 though passes after applying Richard's patch. BTW, I couldn't help but notice in the output of the test case I wrote that a generated

Re: Avoid lost result of recursion (src/backend/optimizer/util/inherit.c)

2022-12-22 Thread Amit Langote
list by the above code block after the buggy multi-level translation in ger_rel_all_updated_cols(). Thanks for writing the patch, Richard. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com v1-0001-postgres_fdw-test-update-of-multi-level-partition.patch Description: Binary data

Re: generic plans and "initial" pruning

2022-12-21 Thread Amit Langote
On Wed, Dec 21, 2022 at 7:18 PM Alvaro Herrera wrote: > This version of the patch looks not entirely unreasonable to me. I'll > set this as Ready for Committer in case David or Tom or someone else > want to have a look and potentially commit it. Thank you, Alvaro. -- Thanks, Amit La

Re: Allow batched insert during cross-partition updates

2022-12-20 Thread Amit Langote
On Tue, Dec 20, 2022 at 7:18 PM Etsuro Fujita wrote: > On Wed, Dec 14, 2022 at 10:29 PM Amit Langote wrote: > > On Wed, Dec 14, 2022 at 6:45 PM Etsuro Fujita > > wrote: > > > One thing I noticed is this bit: > > > > > > -- Clean up > &

Re: generic plans and "initial" pruning

2022-12-15 Thread Amit Langote
On Wed, Dec 14, 2022 at 5:35 PM Amit Langote wrote: > I have moved the original functionality of GetCachedPlan() to > GetCachedPlanInternal(), turning the former into a sort of controller > as described shortly. The latter's CheckCachedPlan() part now only > locks the "min

Re: Allow batched insert during cross-partition updates

2022-12-14 Thread Amit Langote
On Wed, Dec 14, 2022 at 6:45 PM Etsuro Fujita wrote: > On Thu, Dec 8, 2022 at 8:01 PM Etsuro Fujita wrote: > > On Thu, Dec 8, 2022 at 5:00 PM Amit Langote wrote: > > > Updated patch attached. > > > > I will review the patch a bit more, but I think > > it w

Re: generic plans and "initial" pruning

2022-12-14 Thread Amit Langote
On Tue, Dec 13, 2022 at 2:24 AM Alvaro Herrera wrote: > On 2022-Dec-12, Amit Langote wrote: > > I started feeling like putting all the new logic being added > > by this patch into plancache.c at the heart of GetCachedPlan() and > > tweaking its API in kind of unintuitive w

Re: generic plans and "initial" pruning

2022-12-12 Thread Amit Langote
On Fri, Dec 9, 2022 at 8:37 PM Alvaro Herrera wrote: > On 2022-Dec-09, Amit Langote wrote: > > > Pruning will be done afresh on every fetch of a given cached plan when > > CheckCachedPlan() is called on it, so the part_prune_results_list part > > will be discarded a

Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)

2022-12-11 Thread Amit Langote
On Sun, Dec 11, 2022 at 6:25 PM Amit Langote wrote: > I've attached 0001 to remove those extraneous code blocks and add a > comment mentioning that userid need not be recomputed. > > While staring at the build_simple_rel() bit mentioned above, I > realized that this code fail

Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)

2022-12-11 Thread Amit Langote
Hi, On Sun, Dec 11, 2022 at 5:17 AM Justin Pryzby wrote: > On Tue, Nov 29, 2022 at 10:37:56PM +0900, Amit Langote wrote: > > 0002 contains changes that has to do with changing how we access > > checkAsUser in some foreign table planning/execution code sites. > > Thoug

Re: generic plans and "initial" pruning

2022-12-09 Thread Amit Langote
On Fri, Dec 9, 2022 at 7:49 PM Alvaro Herrera wrote: > On 2022-Dec-09, Amit Langote wrote: > > On Fri, Dec 9, 2022 at 6:52 PM Alvaro Herrera > > wrote: > > > Remind me again why is part_prune_results_list not part of struct > > > CachedPlan then? I tried to

Re: generic plans and "initial" pruning

2022-12-09 Thread Amit Langote
On Fri, Dec 9, 2022 at 6:52 PM Alvaro Herrera wrote: > On 2022-Dec-09, Amit Langote wrote: > > On Wed, Dec 7, 2022 at 4:00 AM Alvaro Herrera > > wrote: > > > I find the API of GetCachedPlans a little weird after this patch. > > > David, in his Apr 7 reply on thi

Re: generic plans and "initial" pruning

2022-12-09 Thread Amit Langote
r as I can > tell, the callers that pass a non-NULL pointer there are the exactly > same that later call PortalStorePartitionPruneResults. Yes, it would be better to not need PortalStorePartitionPruneResults. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: Allow batched insert during cross-partition updates

2022-12-08 Thread Amit Langote
Hi Fujita-san, On Tue, Dec 6, 2022 at 6:47 PM Etsuro Fujita wrote: > On Tue, Mar 22, 2022 at 10:17 AM Amit Langote wrote: > > Rebased to fix a minor conflict with some recently committed > > nodeModifyTable.c changes. > > Apologies for not having reviewed the patch.

Re: moving extraUpdatedCols out of RangeTblEntry (into ModifyTable)

2022-12-07 Thread Amit Langote
On Wed, Dec 7, 2022 at 8:54 PM Amit Langote wrote: > Per Alvaro's advice, forking this from [1]. > > In that thread, Tom had asked if it wouldn't be better to find a new > place to put extraUpdatedCols [2] instead of RangeTblEntry, along with > the permission-checking fields ar

Re: moving extraUpdatedCols out of RangeTblEntry (into ModifyTable)

2022-12-07 Thread Amit Langote
On Wed, Dec 7, 2022 at 8:54 PM Amit Langote wrote: > Per Alvaro's advice, forking this from [1]. > > In that thread, Tom had asked if it wouldn't be better to find a new > place to put extraUpdatedCols [2] instead of RangeTblEntry, along with > the permission-checking fields ar

<    1   2   3   4   5   6   7   8   9   10   >