feat: add support for DDL and INSERT/DELETE/UPDATE operations

[curino]

In collaboration with Jesus Camacho Rodriguez

[curino]

Cleaned up git commit log (I spend most of my time fighting this linter!).

[curino]

@jacques-n and others, how does this version look?

[jacques-n]

Hey @curino , this is on my list to review. Will try to get to it soon.

One thing I still don't understand is the output item in writerel. Can you give a concrete example in pseudo code/description?

[curino]

Thanks @jacques-n!

Most DBMS return information about how the insert/delete/update operation went. These takes few forms:

1. No output (default)
2. A simple count of how many tuples were affected by the operation
3. Returning the set of tuples deleted/inserted
4. Returning the before (or after, or both) image of an update
5. An arbitrary computation of the affected tuples

What I learned reading few doc pages from various systems is that this is to simplify the common case where apps want to be able to double check how an operation went (e.g., validate that roughly the right number of tuples were affected) and potentially abort/compensate if things were off. This would otherwise required temp tables and complex multi-table transactions, so it is a rather appreciated/used feature.

Including an output term allows us to capture any of the above scenarios in a compact way. Basically we are telling the systems to use input query to determin which tuples to change and output to report on what was done per user required semantics. In some systems there are special keywords the output query can refer to such as BEFORE and AFTER (think of those as tables capturing the before and after image of the affected tuples). I think it is ok to expect that the system generating the substrait plan produces valid pairs of input and output Rels (some validation is possible, but not very deep).

I considered to simplify this and hardcode the "# of affected tuples" semantics as a return value, but this does seem to miss what I read was a rather common use case. I hope this helps clarify the rationale (happy to chat in the community call if it is easier).

Thanks,
Carlo

[curino]

Per conversation in latest community meeting, I modified the output mechanisms to leverage a spool (via references) for the BEFORE image, and a simple enum NO_OUTPUT or MODIFIED_TUPLES to control the operator output, that we can then process (e.g., with a COUNT) to shape the output of AFTER image changes.

I also realized that in the original docs there were references to a bunch of other things (rotations, etc.) that are not currently represented (or at least I am not clear on what they mean). I would propose we converge on this patch, and then do further iterations to reflects further/fancier things (e.g., we left constraints and indexes out of table definitions as well).

[curino]

@jacques-n ping...

[jacques-n]

This doesn't feel like an operation. It fields like a subcategory of the other operation types.

[curino]

We could have the DDL_OP being TABLE or VIEW and then have another field capturing CREATE, ALTER, DROP (where VIEW + ALTER is equivalent to the CREATE_OR_REPLACE we have now)

[jacques-n]

I'm inclined to just include named table and extension table right now. (With the latter being completely independent of the read type.)

Amongst other things, I'm not convinced that a read spec for files is the same as a write spec.

[curino]

We could include tables only for now, but eventually we want WriteRel to write to file as well I think.

[jacques-n]

This feels like it doesn't belong here. It isn't really specific to a named table, right? Maybe it should be at the write rel level?

[jacques-n]

As noted, if we move ctas, this can also exist inside of writerel. Note that we should specifically declare patterns around the requirement of including this (or not) and the behavior of it. I'd also note that after-image isn't clear here.

[curino]

Even if we move CTAS we need schema (names+types) and defaults for CREATE_TABLE right?

[jacques-n]

I suggest we move CTAS to WriteRel. This removes the write_type stuff entirely from this and keeps it only in WriteRel. (This may mean we need to come up with a different name than DDL since that generally includes CTAS.)

Separately, I think we should probably remove CREATE TABLE and ALTER TABLE since there isn't sufficient information here to actually do them. (We need schema, etc.)

[curino]

If we keep the write_type it contains a NamedTableWrite, which includes all we need for CREATE or ALTER TABLE I think (table name, column names, column types, default values).

[jacques-n]

As mentioned elsewhere, let's move this into WriteRel and keep it constrained to that location.

[curino]

I think this prevents us to do all `CREATE' and 'ALTER' operators.

[jacques-n]

I can't figure out how to use names + base schema to map my partial input onto a broader schema. I also don't think this belongs in Named Table, feels like a generic/cross table concept. After moving properties to writerel, I think we need a better clarity around what this write is and how it maps to an underlying schema. One option:

final schema and a indexbased mapping mapping the input schema to the output. For example:

input: (a int, c int)
output: (a int, b int, c int, d int)
defaults: (1, 4, 7, 6) -- defined based on the output schema.
map: (0,2) maps: input[0] => output[0] and input[1] => output[3]

I can't see how to use a set of names to do this mapping since there could be dupes at various levels of schema. (It also seems anti-substrait-patterns to use some form of naming system to map two schemas).

[jacques-n]

We should also probably have an option around defaults: do we only apply for missing columns or do we also apply for null values. (The latter is a lot more expensive...)

[curino]

The thinking is as follows:

1. names is simply a qualified table name (like mydb.table). I am copying the pattern from ReadRel.NamedTable. Maybe we should call it qualified_table_name or something like that.
2. base_schema describes the entire schema we want in output (after an ALTER or as a result of CREATE or INSERt operation etc.)
3. The content is positionally (as usual) mapped to input. For example if you are doing UPDATE TABLE foo WHERE col1=3 WHERE cond, we would have in the input something like SELECT "3", col2, col3... FROM foo WHERE cond.

In hindsight we might be able to skip the defaults field altogether and use the input for that (using a set of constants if we are issuing a CREATE TABLE for example).

[curino]

Thanks @jacques-n for the live-chat. I have addressed all we discussed, and updated some of the docs accordingly.

[jacques-n]

One little leftover that should be cleaned but otherwise looking good. I'll fix and merge assuming tests pass.

[curino]

Thanks @jacques-n and @jcamachor for the help shaping this.

[jvanstraten]

I'm a bit late to the party now (was on vacation, sorry), but I can't help but notice that oneof rel_type wasn't updated. AFAICT the new messages are entirely unreachable from Plan. Is that intentional at this time? I haven't read any of this in detail yet though, so I might have missed something.