Repository navigation
Conversation
Adds a --column-name-strategy option (and matching Java API method) for import-avro-files, import-delimited-files, import-jdbc, import-orc-files, and import-parquet-files, applied after --where, --drop, and --group-by/--aggregate/--aggregate-order-by so those options can still reference original column names. - none (default): column names are left unchanged, preserving existing behavior. - simple: folds Western European Latin letters to their closest ASCII equivalent (e.g. 'é' -> 'e', 'ö' -> 'o', 'ß' -> 'ss', via Unicode NFD normalization plus an explicit mapping for letters that don't decompose, such as 'æ' -> 'ae'), then replaces each remaining run of non-ASCII-alphanumeric characters (including existing underscores) with a single underscore and trims leading/trailing underscores. Sanitization is applied via a single positional rename so a column's sanitized name colliding with another column's original name doesn't produce a false intermediate collision. Raises a FluxException if sanitization would produce an empty name or a collision between two different columns. Also documents the feature in docs/import/structured-data, and bumps the test task's max heap size to prevent OOM in Spark tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Underscores in the original column name may be meaningful, so the simple strategy no longer collapses or trims them. Only leading/trailing runs of other unsupported characters are removed, and interior runs are replaced with a single underscore. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
stevebio
added this pull request to stack #679
September 29, 2026 16:41
stevebio
requested review from
RitaChen609,
jonmille,
ngodugu-marklogic,
rjdew-progress and
rjrudin
as code owners
September 29, 2026 16:41
rjrudin
requested changes
Sep 29, 2026
rjrudin
left a comment
rjrudin
left a comment
Contributor
There was a problem hiding this comment.
Looks good, just a few comments about moving some furniture around.
Addresses PR #678 review feedback: - Move sanitizing logic into a ColumnNameSanitizer interface with a SimpleColumnNameSanitizer implementation, decoupled from the Dataset API. - Replace Spark-based sanitization tests with unit tests on the sanitizer, keeping one integration test in ImportDelimitedFilesTest. - Drop option-ordering detail from the --column-name-strategy description. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
stevebio
changed the title
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The public API addition affects existing implementations, and the sanitizer can miss duplicate names or silently remove unsupported marks.
Review effort: Balanced
Findings: 1
Open (4)
What changed in this PR
This PR adds optional column-name sanitization to structured-data imports before documents are constructed.
Changes:
- Adds the
noneandsimplestrategies to five import commands and their APIs. - Adds sanitizer, option-parsing, and delimited-import tests.
- Documents the option and sets a 4 GB maximum heap for tests.
| File | Description |
|---|---|
flux-cli/src/test/java/com/marklogic/flux/impl/importdata/SimpleColumnNameSanitizerTest.java |
Tests name conversion and errors. |
flux-cli/src/test/java/com/marklogic/flux/impl/importdata/ImportDelimitedFilesTest.java |
Tests the CLI import option. |
flux-cli/src/test/java/com/marklogic/flux/impl/importdata/ColumnNameStrategyOptionsTest.java |
Tests defaults and option parsing. |
flux-cli/src/test/java/com/marklogic/flux/api/DelimitedFilesImporterTest.java |
Tests API-based sanitization. |
flux-cli/src/main/java/com/marklogic/flux/impl/importdata/StructuredDataParams.java |
Applies sanitization after transformations. |
flux-cli/src/main/java/com/marklogic/flux/impl/importdata/SimpleColumnNameSanitizer.java |
Implements the simple strategy. |
flux-cli/src/main/java/com/marklogic/flux/impl/importdata/ImportParquetFilesCommand.java |
Exposes the Parquet API option. |
flux-cli/src/main/java/com/marklogic/flux/impl/importdata/ImportOrcFilesCommand.java |
Exposes the ORC API option. |
flux-cli/src/main/java/com/marklogic/flux/impl/importdata/ImportJdbcCommand.java |
Exposes the JDBC API option. |
flux-cli/src/main/java/com/marklogic/flux/impl/importdata/ImportDelimitedFilesCommand.java |
Exposes the delimited-file API option. |
flux-cli/src/main/java/com/marklogic/flux/impl/importdata/ImportAvroFilesCommand.java |
Exposes the Avro API option. |
flux-cli/src/main/java/com/marklogic/flux/impl/importdata/ColumnNameSanitizer.java |
Defines the sanitizer interface. |
flux-cli/src/main/java/com/marklogic/flux/api/StructuredDataImporter.java |
Adds the importer API method. |
flux-cli/src/main/java/com/marklogic/flux/api/ColumnNameStrategy.java |
Defines the available strategies. |
docs/import/structured-data/sanitizing-column-names.md |
Documents usage and limitations. |
docs/import/structured-data/overview.md |
Links to the new guide. |
build.gradle |
Sets the test maximum heap. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Keeps existing external implementations of the public interface source- and binary-compatible. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
stevebio
changed the title
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--column-name-strategyto structured-data imports:none(default) preserves existing behavior, whilesimplesanitizes column names.simplestrategy folds Western European letters, replaces unsupported character runs with underscores, and preserves original underscores.ColumnNameSanitizerinterface (SimpleColumnNameSanitizer), decoupled from the Spark Dataset API.Testing
SimpleColumnNameSanitizercovering folding, underscores, collisions, and empty names.