Skip to content

fix(csv): report rows that cannot be imported instead of changing or dropping them - #237

Open
HarshMN2345 wants to merge 3 commits into
2.0.xfrom
fix-csv-import-rows
Open

HarshMN2345 wants to merge 3 commits into
2.0.xfrom
fix-csv-import-rows

Conversation

@HarshMN2345

Copy link
Copy Markdown
Member

CSV import (Sources\CSV):

  • A cell that does not parse as its column type rejects its row with the row number and column (Row 5: 'age' must be an integer.) instead of storing NULL or a cast value. This covers integers (including 1.7 and out-of-range values), floats, booleans, array elements and JSON columns. Whole numbers written as 1.0 or 1e3 still import. Datetime parsing is unchanged and matches the REST API.
  • A row with the wrong number of columns, or with bytes that are not UTF-8, is rejected the same way and the rest of the file is imported. Before, the first aborted the import and dropped the rows already buffered. The second made PDO drop the whole batch without an error.
  • Rejected rows are counted as error. The first 100 are listed in the migration errors.
  • Text keeps its leading and trailing whitespace. Only an unquoted null is NULL, so "null" imports as text. An empty cell is NULL for every column except text, including enum, email, ip and url. null is also NULL for array columns.
  • A UTF-8 byte order mark before the header and blank lines are skipped.

CSV export (Destinations\CSV): the text null is written quoted, so it reads back as text.

Row writes (Destinations\Appwrite):

  • A row counts as success only once its batch is written. A batch the database rejects marks every row in it as error. Before, a 300-row file with one bad value in row 200 stored 200 rows and reported success: 299, error: 1.
  • A batch is written even when its last row is rejected. Before, an invalid $id in the last row dropped every row buffered before it.

Verification

Local Appwrite 2.4.0 on Postgres, with this branch mounted over vendor/utopia-php/migration:

  • Each case from the issue went through POST /v1/migrations/csv/imports and was read back. The table has varchar, integer, float, boolean, datetime, enum, email, varchar[], integer[] and text columns. Results: the 300-row file with one bad value stores 200 rows and reports success: 200, error: 100. Wrong column count in one row stores 299 and reports error: 1. The BOM file keeps its $ids. The Latin-1 file stores the valid row and reports Row 2 is not valid UTF-8.
  • Export and re-import of rows holding "null", NULL, padded text and arrays now gives the same values back.
  • Appwrite e2e testCreateCSVImport*, testCreateCSVExport, testCreateJSONImport*, testAppwriteMigrationDatabasesRow, testAppwriteMigrationRowsOnDuplicate and testAppwriteMigrationOverwriteDropsOrphanColumn pass. The missing-row.csv assertion was updated to the new error.
  • I compared the quoted-cell detection with fgetcsv() on about 600k generated records for , ; \t |. There were no mismatches.
  • Unit suite: 89 tests OK. composer lint and composer check are clean.

Fixes appwrite/appwrite#14333

A CSV import stored NULL or a cast value for cells that do not parse as
their column type, trimmed text, read a quoted "null" as NULL, kept the
UTF-8 byte order mark in the first header, aborted on a row with the
wrong number of columns, and let one non-UTF-8 value drop its whole
batch without an error.

Each such row is now rejected with its row number and counted as an
error, while the rest of the file is imported. Text keeps its
whitespace, only an unquoted null is NULL, and an empty cell is NULL
for every column except text. The export quotes the text "null" so it
reads back unchanged.
Rows were marked as succeeded when buffered, so a batch the database
rejected still counted every row but one as a success. A rejected last
row also skipped the write, dropping the rows buffered before it.

The batch is now written even when its last row is rejected, a failed
write marks every row in it as an error, and row counters are updated
after the write.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant