MySQL Bugs: #102856: Unexpected NULL Fields errors when updating or inserting record

Bug #102856	Unexpected NULL Fields errors when updating or inserting record - MySQL XDevAPI
Submitted:	8 Mar 2021 12:06	Modified:	11 Mar 2021 13:48
Reporter:	Kevin Lau	Email Updates:
Status:	Closed	Impact on me:	None
Category:	Connector for Node.js	Severity:	S1 (Critical)
Version:	8.0.23	OS:	MacOS
Assigned to:	Rui Quelhas	CPU Architecture:	Any

Description:
In the older versions, statement like below was good if passing a null value to it.

`First_Name` VARCHAR(60) NULL"

After upgrading to 8.0.23. If not passing any value or null to update or insert a nullable field, it will have errors such as this.

The error statement: "Error: Column name is required in ColumnIdentifier."
Occurred when passing a null value at update operations.

The error statement: "Error: Wrong number of fields in row being inserted"
Occurred when passing a null value during insert operations. The number of field is correct. The error will go away when given non-null value to a column.

If this is a design decision, when updating a field using NodeJs, I would be required to update all the fields even I want some fields to be unchanged. 

Reference:

https://forums.mysql.com/read.php?44,695055,695250#msg-695250

How to repeat:
If there is a direct correlation between NodeJS connector and MySQL, the new implement is inconsistent to the behavior of MySQL. NodeJS Connector error occurs for the equivalent MySQL statement below since the nodeJS user must provides all fields for each update 
 
MYSQL CODE
CREATE TABLE IF NOT EXISTS tasks (
    task_id INT AUTO_INCREMENT,
    title VARCHAR(255) NOT NULL,
    nullable TINYINT NULL,
    PRIMARY KEY (task_id)
);
INSERT INTO tasks(title, nullable)
VALUES('Learn MySQL INSERT Statement', 1);
UPDATE tasks SET title = 'newTitle' WHERE task_id = 1;
 
NodeJS Code
tasksTb.update().where(`taskid = 1`).set('title', 'newTitle')

Suggested fix:

NodeJs Connector shouldn't throw this error since its function is to connect, not do extra type check on behalf of MySQL. MySQL should have the only say about this matter. 

In practice;
some fields are intended never be changed since created or not updatable for security reason.

Hello Kevin Lau,

Thank you for the report and feedback.
This seems to be intended behavior after Bug #100425 fixed.
Verifying based on the comment from Rui at https://forums.mysql.com/read.php?44,695055,695250#msg-695250

regards,
Umesh

Hi Kevin,

I'm a bit confused by your claim. You say that:

"If this is a design decision, when updating a field using NodeJs, I would be
required to update all the fields even I want some fields to be unchanged."

But the fact that you are able to do something like:

  tasksTb.update().where('taskid = 1').set('title', 'newTitle')

is exactly why it that is not the case, and you get the corresponding SQL query:

  UPDATE tasks SET title = 'newTitle' WHERE task_id = 1;

So, it's not true that you need to update ALL the fields. Using "set()" you update only
the fields you like. Of course, if you want to set some existing field to "NULL" you also
need to be specific about that. Omitting it from the query does exactly what you want when
you say that some fields should remain unchanged.

What I mention by design decision is that "undefined" in JavaScript should not match "NULL" in SQL,
not that the JavaScript API should exactly mimic the SQL API. As a side note, this is exactly what
happens if you use the MySQL Shell JavaScript API, and we also want to keep a similar API on our end.

Maybe I'm confused, but, as far as I can tell, you want to be able to do something like:

  UPDATE tasks SET title = 'newTitle', nullable = NULL WHERE task_id = 1;

And, somehow you want the corresponding API call to allow the following:

  tasksTb.update().where('taskid = 1')
    .set('title', 'newTitle')
    .set('nullable') // which as I' claiming is the same as ".set('nullable', undefined)"
    .execute()

Which leads me to believe your problem is that you want to call ".set('nullable', x)" but
it might be the case that "x" is "undefined". If that is the case you can always do something like
"set('nullable', x || null)".

I understand this is a breaking change and it is annoying, however, I would argue you were considering
something as a "feature", when in fact it was a "bug". It is an unfortunate situation, but we think
the new behavior is a step in the right direction. Which does not mean we are not wrong and it cannot
be reverted.

So, my question is: Is that workaround sufficient for you? If not, can you present a case where it is
not?

Sorry about the confusion, let me clarify, the statement below will produce an error in this new release.
 
tasksTb.update().where('taskid = 1').set('title', 'newTitle')

To fix this error, I would need to do the below to get rid of the error. (Not passing an undefined to the update, I agree to your logic)

tasksTb.update().where('taskid = 1').set('title', 'newTitle').set('nullable', "") or 
tasksTb.update().where('taskid = 1').set('title', 'newTitle').set('nullable', null)

IMO, some of my records don't have all the values available for an update. But I only want to update certain fields. To comply with the new release, I need to have a record with all the values for each field, to update the record otherwise, I need to insert a null value or get an error. Subsequently forcing me to do an extra fetch for all values in this record. 

Thanks for your patience and understanding.

Kevin, I am still a bit confused. 

You have the following table:

+-----------+-------------------------------+-----------+
| task_id   | title                         | nullable  |
+-----------+-------------------------------+-----------+
| 1         | Learn MySQL INSERT Statement  | 1         |
+-----------+-------------------------------+-----------+

And you want to update only the "title" column of the record, so it becomes:

+-----------+-------------------------------+-----------+
| task_id   | title                         | nullable  |
+-----------+-------------------------------+-----------+
| 1         | newTitle                      | 1         |
+-----------+-------------------------------+-----------+

For this to happen, all you need to do is to use that API call you mention:

tasksTb.update()
  .where(`taskid = 1`)
  .set('title', 'newTitle')
  .execute()

This works fine, both in 8.0.22 and in 8.0.23. I am not sure why you say it produces an error. At least, I am not able to reproduce it.

If, you also want to update the "nullable" column (to NULL) in the same operation:

+-----------+-------------------------------+-----------+
| task_id   | title                         | nullable  |
+-----------+-------------------------------+-----------+
| 1         | newTitle                      | NULL      |
+-----------+-------------------------------+-----------+

You need to do a bit more:

tasksTb.update()
  .where(`taskid = 1`)
  .set('title', 'newTitle')
  .set('nullable', null)
  .execute()

And only in this last example you can see exactly what changed between 8.0.22 and 8.0.23. Whereas on 8.0.22 (and previous versions), besides this convention, you could also get away with:

tasksTb.update()
  .where(`taskid = 1`)
  .set('title', 'newTitle')
  .set('nullable')
  .execute()

or 

tasksTb.update()
  .where(`taskid = 1`)
  .set('title', 'newTitle')
  .set('nullable', undefined)
  .execute()

now with 8.0.23, it will, indeed fail with an "Column name is required in ColumnIdentifier." error because you need to always be explicit and use ".set('nullable', null)" instead.

The fact that you use "nullable TINYINT NULL" only tells MySQL that it should allow NULL values, not that somehow it needs to update a column field to NULL if a new value is not specified in the update clause. 

So, I think the workaround is clear, you need to start sending "null" instead of "undefined", which can be easily achieved, as I mentioned before, using an "||" operator to check for the existence of a value and use the second operand if it does not exist.

Maybe I am still missing something, and if that is the case, it would be great if you can give me an explicit example where the workaround does not apply.

Continue with this example, my current code is as follow, it will produce an error. I haven't passed an undefined or null value to the update. Certain fields I never want to make it updatable since the creation of the record.

tasksTb.update()
  .where(`taskid = 1`)
  .set('title', 'newTitle')
  .execute()

To comply with the new release, each update need to have a complete set of values for each field, in the new release, I can't assume some fields want to retain its original values by negating the field in the update statement.

Kevin, I just tested the following code, which matches the details you have provided until now, with Connector/Node.js 8.0.23, and I can't reproduce any error.

const config = {
  schema: 'update_single',
  table: 'tasks'
}

session.sql(`CREATE DATABASE IF NOT EXISTS \`${config.schema}\``)
  .execute()
  .then(() => {
    return session.sql(`CREATE TABLE IF NOT EXISTS \`${config.schema}\`.\`${config.table}\` (task_id INT AUTO_INCREMENT, title VARCHAR(255) NOT NULL, nullable TINYINT NULL, PRIMARY KEY (task_id))`)
      .execute()
  })
  .then(() => {
    return session.sql(`INSERT INTO \`${config.schema}\`.\`${config.table}\` (title, nullable) VALUES ('Learn MySQL INSERT Statement', 1)`)
      .execute()
  })
  .then(() => {
    return session.getSchema(config.schema).getTable(config.table).select()
      .execute(console.log)
  })
  .then(() => {
    return session.getSchema(config.schema).getTable(config.table).update()
      .where('task_id = 1')
      .set('title', 'newTitle')
      .execute()
  })
  .then(() => {
    return session.getSchema(config.schema).getTable(config.table).select()
      .execute(console.log)
  })

The output is the expected:

[ 1, 'Learn MySQL INSERT Statement', 1 ]
[ 1, 'newTitle', 1 ]

Again, unless you give me a specific reproducible example where it doesn't work, I won't be able to help a lot more.

Problem resolved.