Revise the mutations API

[soupi]

What

We want to make the api for mutations pretty. We make the following changes:

1. Rename arguments
2. prefix the argument names of unique constraint columns with key_

How

General design

• We generate delete, insert and update procedures for each table.
• A single insert procedure is generated per table of the form:

  <mutation-version>_insert_<table>(
      objects: [<object>],
      post_check: <boolexpr>
  )

  It lets us insert multiple objects and include a post check for permissions.
• A delete procedure is generated per table X unique constraint of the form:

  <mutation-version>_delete_<table>_by_<column_and_column_and...>(
      key_<column1>: <value>,
      key_<column2>: <value>,
      ...,
      pre_check: <boolexpr>
  )

  It lets us delete a single row using the uniqueness constraint, and contains a pre check for permissions.
• An update procedure is generated per table X unique constraint of the form:

  <mutation-version>_update_<table>_by_<column_and_column_and...>(
      key_<column1>: <value>,
      key_<column2>: <value>,
      ...,
      update_columns: { <column>: { _set: <value> }, ... },
      pre_check: <boolexpr>,
      post_check: <boolexpr>
  )

  It lets us update a single row using the uniqueness constraint by updating the relevant columns, and contains a pre check and post check for permissions.
  Note that the update_columns structure is different than the v2 version where we had _set, _inc, and other fields.
• Mutations using uniqueness constraints use the naming schema by_column_and_column_and_column instead of the db constraint name, because the former is far more helpful.
• If generating a mutation encounters an internal error, we skip that particular mutation instead of throwing an error so the connector can start at any situation.
• Naming collisions between the unique constraints and the update_columns / pre_check / post_check is avoided by prefixing argument names of the columns of a unique constraint with key_.

[soupi]

so we can specify the name once instead of having magic strings.

[SamirTalwar]

Perhaps the arguments map should use a tuple or something as a key?

[soupi]

arguments comes from ndc-spec, and they are not used only for keys, but also for update_columns, pre_check, etc.

I agree that this isn't looking great, but I'm not sure there's a nicer solution.

[SamirTalwar]

Got it, makes sense.

[SamirTalwar]

Suggested change

	.ok_or(Error::ArgumentNotFound(pre_check.argument_name.clone()))?;
	.ok_or_else(|| Error::ArgumentNotFound(pre_check.argument_name.clone()))?;

[SamirTalwar]

Why isn't this in the Display impl of UnexpectedStructure?

[soupi]

Your comment made sense to me. I went to have a look what would I need to parameterize UnexpectedStructure by to make this work, and the result was:

• array of arrays
• insert
• {} argument
• an array of objects

Which is essentially almost the entire message :( Might not be worth trying to parameterize in that case, and we probably don't need a dedicated error time for each unexpected structure.

[SamirTalwar]

Suggested change

	.ok_or(Error::ArgumentNotFound(
	.ok_or_else(|| Error::ArgumentNotFound(

[soupi]

What is the difference?

[soupi]

Went to have a look. It seems like .ok_or_else is useful when the or_else part is an expensive computation. This isn't the case here - we need to create a closure that returns this particular data, I think this will end up doing more work and require more memory, won't it? So isn't it strictly worse than just ok_or in this case?

[SamirTalwar]

It's just because of the .clone(); I think it's probably borderline.