Missing WHERE clause in Bulk APIs

[fishnux]

This is a critical bug because, even if the query succeeds, it'll update rows you don't want to update (all of them)

NocoDB used as docker : true
NocoDB version : 0.84.14
Database used in NC_DB URL : sqlite3
Project was created by clicking : New Project by connecting to external database
Database on which spreadsheet is created : pg
OS on which NocoDB is running : Linux
Node.js version if running as node : N/A
Database version :

Steps To Reproduce
Do REST API request to bulk update, e.g.

[
    {
        "id": 2,
        "email": "example@example.com"
    },
    {
        "id": 3,
        "email": "example2@example.com"
    }
]

(please note I haven't literally tested the example above, I found this bug within n8n's NocoDB integration)

Expected behavior
Bulk update succeeds

What happens

error: update "mytable" set "id" = $1, "email" = $2 - duplicate key value violates unique constraint "mytable_pkey"

It's missing the WHERE clause for UPDATE - all rows would be updated if the query succeeds

[wingkwong]

Applying to Delete as well.

[fishnux]

@wingkwong you don't think this is a bug? Because the bulk API is implemented, and is clearly misbehaving by not having the "where" clause. You do realize people might use the bulk API for updating data, and it would update data in an erratic manner? Requiring a restore from a database backup because the bulk update went the opposite way?

This issue being classified as "Enhancement" instead of a "Bug" is something I don't understand. I kindly ask you to re-consider for the sake of other users that might be using the bulk update API unaware of this bug

[wingkwong]

@fishnux Some users trigger a bulk API resetting a column for all records such as updating a boolean column as false. It's not misbehaving in this case. I agree that adding where clause would be nice as some users may want to apply the changes on some target records only. As something isn't working the way the customer wants, then it's classified as an enhancement. If users cannot update records at all, then it's classified as a bug.

Even there is an option to specify where clause, to use it or not, it's on users. They can still update / delete all records by mistake when they leave where clause empty. Of course there could be a lot to be done, like notifying how many records are gonna be updated before executing the SQL. In this case, I'd say this is another potential enhancement rather than a bug.

[fishnux]

Ok, I understand it now, thanks for explaining! Looks like it depends on the use-case

[fishnux]

Actually, how do you explain this in the documentation:
https://docs.nocodb.com/developer-resources/rest-apis#bulk-update

[
    {
        "country_id" : 10261,
        "country": "test 3"
    },
    {
        "country_id" : 10262,
        "country": "test 4"
    }
]

There would be no "test 3", everything would be "test 4", right? @wingkwong

[wingkwong]

@fishnux Just checked with another developer. The origin design is that if the primary key is mentioned, country_id in the example, then it will be added to where clause. Ideally, country will be updated to test 3 for the record with country_id = 10261 and test 4 for the record with country_id = 10262. However, if the second object doesn't contain country_id, then it'd override all records. After an internal discussion, we'd have multiple enhancements on Bulk APIs. Please stay tuned.

[D-side]

I've hit the same issue last summer with deletion in PostgreSQL, which wiped my whole table (I had backups, so no data loss 👌). But the project I was on was really low on my priority list, so I never ended up narrowing it down or reporting it. Sorry about that and kudos to @fishnux for making it happen!

Great to see the reasoning behind this design. On a technical level it kinda makes sense, but I have to agree that it's mighty confusing and very error-prone.

My suggestion would be to disallow objects without primary key in bulk updates, and if the feature "update everything" is indeed necessary, introduce a special value to trigger this behavior.

• Like "country_id": "*" — which is less than ideal though in case primary key is a string, which could make "*" a possible value there.
• Or "country_id": null — primary key cannot be null, so this case can be assigned special significance as long as it's explicitly present in the input (i. e. not using nil-punning to have null possibly mean both absence or being set to nothing).
• Or this could be indicated using a special key if neither solution with a value looks great, although at a glance this would be even uglier both in implementation and API ease-of-use. Sadly, there aren't that many valid types in JSON to work with ☹

Whatever you decide, looking forward to the fix and thanks for such a great tool! 👍

[wingkwong]

In our previous internal discussion, we had the following design.

If the input object length is just 1, then users have to tick the checkbox in swagger ui (or pass a param flag) as a confirmation in order to update all records.

Example: All values in column country will be updated

[
  {
     "country": "VALUE_FOR_ALL_RECORDS"
  }
]

If the PK is provided, then only update that record with the given PK.

Example: Only record with country_id=10261 will be updated

[
  {
    "country_id" : 10261,
    "country": "VALUE_FOR_COUNTRY_ID_EQUAL_TO_10261"
  }
]

If there is multiple objects, for each object, we only update those with PK only. We'd also make it more flexible so that each object is independent as some users may want to update different fields for different IDs.

Example: Only records with country_id=10261 or country_id=10262 will be updated

[
  {
    "country_id" : 10261,
    "country": "VALUE_FOR_COUNTRY_ID_EQUAL_TO_10261"
  },
  {
    "country_id" : 10262,
    "other_field": "VALUE_FOR_COUNTRY_ID_EQUAL_TO_10262"
  },
  {
    "country": "THIS_WILL_NOT_BE_UPDATED"
  }
]

For composite PKs, you need to specify like this.

Example: Record with composite PK 1__2__3 will be updated.

[
  {
    "composite_key_1" : 1,
    "composite_key_2" : 2,
    "composite_key_3" : 3,
    "country": "VALUE"
  }
]

Pseudocode:

if len(input) is 1 
  if pk is present
     updateOne()
  else 
    if query.params.bulk is true
      updateAll()
    else 
      do nothing
else 
  for each object
     if pk is present
        updateOne()
     else 
       do nothing

[D-side]

After thinking about it some more I think I'd rather move the updateAll() part into a separate endpoint, PUT .../api/v1/Table, that accepts a single object of changes (not an array because there's no point) and optionally a where filter like GET .../api/v1/Table (aka "get list") does (consistency! 😊).

And on a side note: what the update currently does looks more like PATCH to me rather than PUT. I rarely see these used properly to be fair, but maybe you'd consider that important. 🙂

[wingkwong]

After thinking about it some more I think I'd rather move the updateAll() part into a separate endpoint, PUT .../api/v1/Table, that accepts a single object of changes (not an array because there's no point) and optionally a where filter like GET .../api/v1/Table (aka "get list") does (consistency! 😊).

After an internal discussion, we'd have the following design.

• update/:id - take a single object
• update/bulk - take an array - each object will be executed independently (if PK is present) so that users could update different different records with different fields in one go
• update/all - take a single object with optional WHERE clause

And on a side note: what the update currently does looks more like PATCH to me rather than PUT. I rarely see these used properly to be fair, but maybe you'd consider that important. 🙂

Yes. We've noticed that and will use PATCH instead in the future.

[wingkwong]

Bulk APIs have been implemented in v0.90.x.

Extracted from Data APIs.

Some sample examples:

Bulk Insert

curl -X 'POST' \
http://localhost:8080/api/v1/db/data/bulk/noco/sakila/testApi \
-H "Content-Type: application/json" \
-H 'accept: application/json' \
-H 'xc-auth: <REDACTED>' \
--data '[{ "Title": "Q" },{ "Title": "P" }]'

Bulk Update

curl -X 'PATCH' \
http://localhost:8080/api/v1/db/data/bulk/noco/sakila/testApi \
-H "Content-Type: application/json" \
-H 'accept: application/json' \
-H 'xc-auth: <REDACTED>' \
--data '[{ "Id": 7, "Title": "Q" },{ "Id": 8, "Title": "P" }]'

Bulk Delete

curl -X 'DELETE' \
http://localhost:8080/api/v1/db/data/bulk/noco/sakila/testApi \
-H "Content-Type: application/json" \
-H 'accept: application/json' \
-H 'xc-auth: <REDACTED>' \
--data '[{ "Id": 27 },{ "Id": 28 }]'

Bulk Update All

curl -X 'PATCH' \
http://localhost:8080/api/v1/db/data/bulk/noco/sakila/testApi/all \
-H "Content-Type: application/json" \
-H 'accept: application/json' \
-H 'xc-auth: <REDACTED>' \
--data '{ "Title": "Updated" }'

Bulk Update All with conditions

curl -X 'PATCH' \
http://localhost:8080/api/v1/db/data/bulk/noco/sakila/testApi/all?where=(Id,gt,7) \
-H "Content-Type: application/json" \
-H 'accept: application/json' \
-H 'xc-auth: <REDACTED>' \
--data '{ "Title": "Updated" }'

Bulk Delete All

curl -X 'DELETE' \
http://localhost:8080/api/v1/db/data/bulk/noco/sakila/testApi/all \
-H "Content-Type: application/json" \
-H 'accept: application/json' \
-H 'xc-auth: <REDACTED>'

Bulk Update All with conditions

curl -X 'DELETE' \
http://localhost:8080/api/v1/db/data/bulk/noco/sakila/testApi/all?where=(Id,gt,7) \
-H "Content-Type: application/json" \
-H 'accept: application/json' \
-H 'xc-auth: <REDACTED>