On Fri, Jan 20, 2017 at 2:49 AM, Kevin Grittner <kgri...@gmail.com> wrote:
> Attached is v9 which fixes bitrot from v8. No other changes.
>
> Still needs review.
This patch still applies, builds cleanly after a small modification,
includes regression tests and the tests past. The modification I
needed to make was due to this compile error:
nodeNamedtuplestorescan.c:154:19: error: no member named
'ps_TupFromTlist' in 'struct PlanState'
scanstate->ss.ps.ps_TupFromTlist = false;
Commit ea15e18677fc2eff3135023e27f69ed8821554ed got rid of that member
of PlanState and I assume based on other changes in that commit that
the thing to do was simply to remove that line. Having done that, it
builds cleanly.
+INSERT INTO level1_table(level1_no)
+ SELECT generate_series(1,200);
+INSERT INTO level2_table(level2_no, parent_no)
+ SELECT level2_no, level2_no / 50 + 1 AS parent_no
+ FROM generate_series(1,9999) level2_no;
+INSERT INTO all_level_status(level, node_no, status)
+ SELECT 1, level1_no, 0 FROM level1_table;
+INSERT INTO all_level_status(level, node_no, status)
+ SELECT 2, level2_no, 0 FROM level2_table;
+INSERT INTO level1_table(level1_no)
+ SELECT generate_series(201,1000);
+DELETE FROM level1_table WHERE level1_no BETWEEN 201 AND 1000;
+DELETE FROM level1_table WHERE level1_no BETWEEN 100000000 AND 100000010;
+SELECT count(*) FROM level1_table;
+ count
+-------
+ 200
+(1 row)
Was it intentional that this test doesn't include any statements that
reach the case where the trigger does RAISE EXCEPTION 'RI error'?
>From the output generated there doesn't seem to be any evidence that
the triggers run at all, though I know from playing around with this
that they do:
postgres=# delete from level1_table where level1_no = 42;
ERROR: RI error
CONTEXT: PL/pgSQL function level1_table_ri_parent_del_func() line 6 at RAISE
+ * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
These copyright messages are missing 3 years' worth of bugfixes.
+SPI_get_caller_relation(const char *name)
Do we need this function? It's not used by the implementation. If it
does have a good use for end-users, then perhaps it should be called
something like SPI_get_registered_relation, to make it clear that it
will return whatever you registered with SPI_register_relation,
instead of introducing this 'caller' terminology?
+typedef struct NamedTuplestoreScan
+{
+ Scan scan;
+ char *enrname;
+} NamedTuplestoreScan;
Nearly plan node structs always have a comment for the members below
'scan'; I think this needs one too because 'enrname' is not
self-explanatory.
/*
+ * Capture the NEW and OLD transition TABLE tuplestores (if specified for
+ * this trigger).
+ */
+ if (trigdata->tg_newtable || trigdata->tg_oldtable)
+ {
+ estate.queryEnv = create_queryEnv();
+ if (trigdata->tg_newtable)
+ {
+ Enr enr = palloc(sizeof(EnrData));
+
+ enr->md.name = trigdata->tg_trigger->tgnewtable;
+ enr->md.tupdesc = trigdata->tg_relation->rd_att;
+ enr->md.enrtuples = tuplestore_tuple_count(trigdata->tg_newtable);
+ enr->reldata = trigdata->tg_newtable;
+ register_enr(estate.queryEnv, enr);
+ SPI_register_relation(enr);
+ }
Why do we we have to call register_enr and also SPI_register_relation here?
On Mon, Dec 19, 2016 at 4:35 AM, Thomas Munro
<thomas.mu...@enterprisedb.com> wrote:
> On Sun, Dec 18, 2016 at 3:15 PM, Kevin Grittner <kgri...@gmail.com> wrote:
>> There were a few changes Thomas included in the version he posted,
>> without really delving into an explanation for those changes. Some
>> or all of them are likely to be worthwhile, but I would rather
>> incorporate them based on explicit discussion, so this version
>> doesn't do much other than generalize the interface a little,
>> change some names, and add more regression tests for the new
>> feature. (The examples I worked up for the rough proof of concept
>> of enforcement of RI through set logic rather than row-at-a-time
>> navigation were the basis for the new tests, so the idea won't get
>> totally lost.) Thomas, please discuss each suggested change (e.g.,
>> the inclusion of the query environment in the parameter list of a
>> few more functions).
>
> I was looking for omissions that would cause some kind of statements
> to miss out on ENRs arbitrarily. It seemed to me that
> parse_analyze_varparams should take a QueryEnvironment, mirroring
> parse_analyze, so that PrepareQuery could pass it in. Otherwise,
> PREPARE wouldn't see ENRs. Is there any reason why SPI_prepare should
> see them but PREPARE not?
Any thoughts about that?
More soon.
--
Thomas Munro
http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
