Skip to content

Correctness fixes found in the pre-release review #14

Description

@vaceslav

From the pre-release review (seven independent reviewers). Each item was confirmed with a reproducer; the first three were re-checked by hand.

Wrong or silently corrupted values

  • Mis-grouped numbers import as other numbers. Under a German plan 19.99 imports as 1999 and 1.5 as 15 (ValueReading.cs parses with NumberStyles.Number / AllowThousands), while the profiler's HasWellFormedGroups refuses them and the precheck blocks them. The import must apply the same well-formed-grouping rule as the profiler.
  • ImportPolicy.AllOrNothing is ignored by the run. Only MappingPrecheck reads TargetSchema.Policy; extractor and importer import partially. Either enforce it (refuse the run / stop at the first failing row) or move the setting to where it is honoured.
  • Precheck is blind to native xlsx numbers mapped to a Date/Boolean/Integer field (MappingPrecheck.cs returns early for natively typed columns): it passes, then every row fails — or, under de-DE, native 1.5 goes through the text fallback and becomes 15.
  • Zoned ISO dates are shifted to the host's time zone. …Z / +05:00 text is parsed with default styles in XlsxCursor (t="d") and DateReading, so one file yields different dates on different servers. Keep wall-clock (or RoundtripKind + documented rule).
  • HeaderRowIndex counts present <row> elements while every reported row number uses the sheet's r; with sparse rows a header on spreadsheet row 3 skips the header and the first data row. Decide one meaning (spreadsheet row) and use it everywhere.

Failure modes

  • Malformed XML throws XmlException from every XmlReader path (workbook, styles, relationships, shared strings — straight out of the constructor). Not documented; a server catching the documented types crashes on a hostile upload. Wrap into the library's exception (see the API issue).
  • A truncated file reads as a shorter, plausible table. When the stream ends mid-row/cell the scanner returns a clean end, and the cursor emits a corrupted final row without any signal. Detect an unterminated document at EOF and fault.
  • A poisoned xlsx cursor stays poisoned after MoveToSheet reopens a sheet from the start: it returns true, then ReadRow throws "cannot continue". Clear the fault when a sheet is reopened, or make MoveToSheet refuse.
  • .xls / .xlsb / other binary files are sent to the csv cursor (TabularFile.Detect calls every non-zip a csv; an xlsb zip fails with "workbook.xml missing"). Decided: reject both with a clear "format not supported" error; also refuse obvious binaries instead of profiling them as text.
  • AnalysisOptions.Cultures = [] throws IndexOutOfRangeException inside the profiler — validate options.
  • Negative / zero r row numbers are accepted.

Needs a decision

  • Invariant globalization. Under DOTNET_SYSTEM_GLOBALIZATION_INVARIANT=1 the default cultures (de-DE, en-US) do not exist and 168 of 394 tests fail. Degrade gracefully (skip unavailable cultures, report it) and document it.
  • German-leaning defaults (Cultures = ["", "de-DE", "en-US"], ; first in delimiter candidates) — keep, but state them in the README/guide.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions