Skip to content

All: Remove deprecated methods for 1.12.0#16449

Open
dramaticlly wants to merge 5 commits into
apache:mainfrom
dramaticlly:1.12deprecation
Open

All: Remove deprecated methods for 1.12.0#16449
dramaticlly wants to merge 5 commits into
apache:mainfrom
dramaticlly:1.12deprecation

Conversation

@dramaticlly
Copy link
Copy Markdown
Contributor

@dramaticlly dramaticlly commented May 20, 2026

Remove all methods, classes, and fields marked with "@deprecated will be removed in 1.12.0" across all modules, since 1.11.0 has been released.

Deleted Classes

  • SystemProperties → use SystemConfigs
  • PartitionStats → use PartitionStatistics interface / BasePartitionStatistics
  • DataReader → use PlannedDataReader
  • GenericAppenderFactory → use GenericFileWriterFactory
  • BaseFileWriterFactory → use RegistryBasedFileWriterFactory
  • S3SignRequest, S3SignResponse, S3SignRequestParser, S3SignResponseParser, S3ObjectMapper → use RemoteSign* equivalents

Notable Non-Mechanical Changes

Beyond deleting deprecated code, the following adjustments were needed:

  1. PositionDelete.set(path, pos, row) and row() — Kept without @Deprecated. Although the public deprecation notice said position deletes with row data are no longer
    supported, internal code (BaseTaskWriter, BasePositionDeltaWriter, RewriteTablePathUtil, Avro) still relies on these methods.

  2. BaseScan.io() — Kept as a non-deprecated protected method. Internal subclasses (DataScan, BaseDistributedDataScan) use it; the deprecation was for external
    callers.

  3. HadoopFileIO.serializeConfWith() — Kept without @Deprecated because it implements the HadoopConfigurable interface contract. Also inlined constructor logic since
    the removed constructor was the delegation target.

  4. BaseParquetReaders.createStructReader(List, StructType, Integer) — Changed from a concrete method (that delegated to the removed 2-param version) to abstract. Both
    existing subclasses (InternalReader, GenericParquetReaders) already override it.

  5. RewriteTablePathUtil.PositionDeleteReaderWriter.writer() — The 4-arg method was a default delegating to the removed 5-arg version. Made it abstract; updated all
    implementors (RewriteTablePathSparkAction in v3.5/v4.0/v4.1) to implement the 4-arg signature directly.

  6. S3V4RestSignerClient — Removed deprecated property fallback logic from baseSignerUri(), endpoint(), and check(). The legacy properties (s3.signer.uri,
    s3.signer.endpoint) are no longer supported.

  7. Parquet.java — Removed the NETFLIX_UNSAFE_PARQUET_ID_FALLBACK_ENABLED branch; without the config, always use NameMapping.empty() (the safe default).

  8. MetricsConfig.forPositionDelete(Table) — All callers (FlinkAppenderFactory, RegistryBasedFileWriterFactory) updated to use the no-arg forPositionDelete() since
    row-level position delete metrics are no longer supported.

  9. GenericFileWriterFactory — Re-added writerProperties to the constructor and Builder since it was a public non-deprecated setter used by GenericAppenderHelper and
    kafka-connect RecordUtils.

// TODO change to required in 1.12.0
if (!properties().containsKey(S3_SIGNER_ENDPOINT)
&& !properties().containsKey(RESTCatalogProperties.SIGNER_ENDPOINT)) {
LOG.warn(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't be removed. Instead, this should go through a Preconditions.checkArgument check

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I think RESTCatalogProperties.SIGNER_ENDPOINT is replaced with RESTCatalogProperties.SIGNER_URI and we can rely on LINE 206 to ensure its existence. Sorry it's a bit messy right now

@github-actions github-actions Bot removed the ORC label May 23, 2026
@dramaticlly dramaticlly force-pushed the 1.12deprecation branch 2 times, most recently from ab4da8d to aa6430d Compare May 23, 2026 00:19
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dramaticlly and others added 4 commits May 25, 2026 12:04
…DeleteRowSchema

- TestS3FileIOProperties: Add required signer.endpoint property since the
  deprecated default was removed
- TestSparkWriterMetrics: Override checkRowStatistics and
  checkNotExistingRowStatistics to match behavior without
  positionDeleteRowSchema (consistent with Flink tests)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
BaseScan.io() and PositionDelete.set(path, pos, row)/row() still have
callers and cannot be removed yet. Restore their deprecation annotations
to preserve the original removal intent.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…io()

The method had only 4 internal callers (DataScan, BaseDistributedDataScan)
which are trivially replaced with table().io(). Also moves revapi entries
to the 1.12.0 section with unified justification.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@dramaticlly dramaticlly marked this pull request as ready for review May 26, 2026 06:35
@dramaticlly
Copy link
Copy Markdown
Contributor Author

@nastra do you want to take another look as now CI is green. I leave the PositionDelete.set(path, pos, row) and row() as is given there's nontrivial amount of caller still use the @deprecated method, plan to handle in a follow up PR

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants