feat: proto change to support WRITE_OP_UPDATE in WriteRel

[scgkiran]

No description provided.

[EpsilonPrime]

Looks good to me. As this is a modification it will require one more approval besides mine.

[westonpace]

The site docs describe the input property for WriteRel as:

The Rel representing which records we will be operating on (e.g., VALUES for an INSERT, or which records to DELETE, or records and after-image of their values for UPDATE).

I interpreted this to suggest that the input to a WriteRel contained the new values. If the new values were defined (in the SQL) as expressions then they would be converted to a project expression before the update.

I'd rather not have two ways to do the same thing so if this is the approach we want then can we fix up the site docs? Also, in doing so I think this would become a breaking change.

I suppose another way to frame the question is if the goal is to more closely mimic SQL or more closely mimic the physical plans that execute the update.

Also, would be good to get some verification on the original intent from @curino or @jacques-n (especially if this is in use somewhere)

[EpsilonPrime]

It's probably worth looking at the SQL that this is supposed to enable.

UPDATE table_name
SET column1 = value1, column2 = value2, ...
WHERE condition;

table_name and the WHERE clause specify what is to be changed (along with the actual values.

The existing behavior is a complete replacement. (Essentially delete all existing rows and add this input.). We should strive to only update the rows that are changing if possible.

[jacques-n]

Discussed this some on sync call today. I think there are two different patterns we should support. The current one is good if you assume implicit row ids (a physical concept).

I suggest we introduce a separate relation which maps more logically to sql.

[jacques-n]

I'm going to close this PR as probably not the optimal changes.