Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .ai/skills/review-comet-iceberg-write-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ bar, and the output format. This skill only covers Iceberg writes.
| Doc | What you need from it |
| ------------------------------------------------------ | --------------------------------------------------------------------------------------- |
| `docs/source/contributor-guide/iceberg-writes.md` | The two layers, the gate's properties, the wire format both ways, cleanup ownership |
| `docs/source/user-guide/latest/iceberg-writes.md` | The eligibility table and the accepted divergences from iceberg-java, as users see them |
| `docs/source/user-guide/latest/iceberg.md` (Writes) | The eligibility table and the accepted divergences from iceberg-java, as users see them |
| `docs/source/contributor-guide/iceberg-spark-tests.md` | What the Iceberg Spark test diffs enable, and what a green run does and does not prove |

The organizing rule is that **the native writer produces what iceberg-java would have produced, or
Expand Down Expand Up @@ -123,7 +123,7 @@ only the latest.
these transforms back to iceberg-rust, or that makes the kernels read the column's timezone
label, breaks that agreement. iceberg-rust's `years` and `months` follow the label
([apache/iceberg-rust#3142](https://github.com/apache/iceberg-rust/issues/3142)). Ask for a
test in a non-UTC session for a change here. "Partition values" in `iceberg-writes.md` lists
test in a non-UTC session for a change here. "Partition values" in the Writes section of the user-guide `iceberg.md` lists
where iceberg-rust differs, and `timezones.md` covers how Comet labels timestamps.
- [ ] **Partition paths use the Java renderers.** Directory names come from
`CometLocationGenerator` / `partition_to_path` in `iceberg_partition_path.rs`, which follow
Expand Down Expand Up @@ -241,9 +241,10 @@ Ask specifically:
properties, the proto description, the native adaptations list, the payload columns and the JVM
steps after them, the cleanup ownership table, the test table, and the Iceberg version per Spark
profile
- **`user-guide/latest/iceberg-writes.md`**: the eligibility table, the "falls back" lists, the
- **`user-guide/latest/iceberg.md`, Writes section**: the eligibility table, the "falls back" lists, the
failure-handling section, and the accepted-divergences list. A PR that lifts or adds a rule, or
changes a divergence, must update these. A divergence the code has since fixed must be removed.
- **`user-guide/latest/iceberg.md`**: its unsupported-features list mentions writes
- **`user-guide/latest/iceberg.md`, Native Reader section**: its unsupported-features list mentions writes. The
Known Issues section lists the open write issues
- **`contributor-guide/iceberg-spark-tests.md`**: the flags the diffs set
- `CometConf.scala` doc strings for the two flags, when defaults or scope change
20 changes: 20 additions & 0 deletions docs/source/_templates/redirect.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->

<html><head><noscript><meta http-equiv="refresh" content="0; url=${to_uri}" /></noscript><script>var target = "${to_uri}";if (window.location.hash) {window.location.replace(target.split("#")[0] + window.location.hash);} else {window.location.replace(target);}</script></head></html>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Non-blocking: with the old hash replacing the target's, a deep link only lands on its section when the old anchor still exists on the new page. For the three Parquet redirects, most old anchors don't:

  • compatibility/scans.html: #schema-mismatch-handling survives; #parquet-scan-limitations doesn't.
  • tuning/scans.html: #filter-pushdown--late-materialization survives; #parquet-reader-tuning, #parquet-native-scans and #iceberg-scan-tuning don't.
  • parquet-iceberg-known-issues.html: only #differences-that-are-not-comet-bugs survives; #parquet-reads, #iceberg-writes, #results-that-differ-from-spark and the rest don't.

For those, the browser ignores the unknown fragment and the reader lands at the top of parquet.html, not at #reading, #tuning or #known-issues.

Letting the target's anchor win when it has one, and carrying the old hash over only when it doesn't, would send every Parquet deep link to its section. iceberg-writes.html would still carry its anchors through, since its target has none and they all still exist:

var target = "${to_uri}";
if (window.location.hash && target.indexOf("#") < 0) { target += window.location.hash; }
window.location.replace(target);

The cost is that the two surviving Parquet anchors land on their parent section instead. The known-issues page was only up for a day, so this mostly matters for the older scans.html links. Fine either way.

2 changes: 1 addition & 1 deletion docs/source/about/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ Common causes of a small speedup are:

Not yet. Comet accelerates [Apache Iceberg](../user-guide/latest/iceberg.md) tables, with a native
reader that is enabled by default and opt-in
[native writes](../user-guide/latest/iceberg-writes.md). It does not accelerate scans of Delta Lake,
[native writes](../user-guide/latest/iceberg.md#writes). It does not accelerate scans of Delta Lake,
Hudi, or Paimon tables, so Spark reads those. Native Delta Lake reads are in development; see the
[roadmap](../contributor-guide/roadmap.md#delta-lake-support). Hudi and Paimon are not on the
roadmap.
Expand Down
2 changes: 1 addition & 1 deletion docs/source/about/gluten_comparison.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ native integrations for Delta Lake, Hudi, or Paimon. See the [Comet Iceberg guid
features and known limitations.

[Comet Iceberg guide]: /user-guide/latest/iceberg.md
[Comet Iceberg writes]: /user-guide/latest/iceberg-writes.md
[Comet Iceberg writes]: /user-guide/latest/iceberg.md#writes

Gluten ships dedicated modules for Iceberg, Delta Lake (2.3 through 4.1, depending on the Spark version), Hudi, and
Paimon. Its [Iceberg documentation] says that reads of unpartitioned tables are offloaded, including tables with
Expand Down
9 changes: 9 additions & 0 deletions docs/source/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@
# enable nice rendering of checkboxes for the task lists
myst_enable_extensions = ["colon_fence", "deflist", "tasklist"]

# Like the extension's default template, but when the old URL carries an anchor, the anchor
# replaces any anchor in the target, so deep links into a moved page still land on their section.
redirect_html_template_file = "_templates/redirect.html"

redirects = {
"overview.html": "index.html",
"gluten_comparison.html": "about/gluten_comparison.html",
Expand All @@ -205,6 +209,11 @@
"user-guide/tuning.html": "latest/tuning.html",
# The compatibility guide became a directory of pages in #4055.
"user-guide/latest/compatibility.html": "compatibility/index.html",
# Parquet and Iceberg documentation was consolidated into one page each.
"user-guide/latest/compatibility/scans.html": "../parquet.html#reading",
"user-guide/latest/compatibility/parquet-iceberg-known-issues.html": "../parquet.html#known-issues",
"user-guide/latest/tuning/scans.html": "../parquet.html#tuning",
"user-guide/latest/iceberg-writes.html": "iceberg.html",
}


Expand Down
4 changes: 2 additions & 2 deletions docs/source/contributor-guide/data-source-audits/iceberg.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

> Audit notes for the native Iceberg scan and the native Iceberg writer. Absence of an entry means
> the area has not been audited yet. Open issues are listed for users in
> [Known Issues in Parquet and Iceberg Reads and Writes].
> [Known Issues in the Iceberg guide].

## Native Iceberg scan

Expand Down Expand Up @@ -86,7 +86,7 @@
- **Not covered:** merge-on-read writes (`WriteDelta`), format v3 types, encryption, object stores,
and write throughput, which needs a release build.

[Known Issues in Parquet and Iceberg Reads and Writes]: ../../user-guide/latest/compatibility/parquet-iceberg-known-issues.md
[Known Issues in the Iceberg guide]: ../../user-guide/latest/iceberg.md#known-issues
[apache/iceberg#17320]: https://github.com/apache/iceberg/pull/17320
[apache/iceberg#17560]: https://github.com/apache/iceberg/pull/17560
[apache/iceberg#17561]: https://github.com/apache/iceberg/issues/17561
Expand Down
5 changes: 3 additions & 2 deletions docs/source/contributor-guide/data-source-audits/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ audits of expressions. Each page holds dated entries per component. An entry rec
- what the audit did not cover

A later audit of the same area adds a new dated entry instead of editing an old one, so the history
stays visible. The open issues that users can hit are listed in the user guide, in
[Known Issues in Parquet and Iceberg Reads and Writes](../../user-guide/latest/compatibility/parquet-iceberg-known-issues.md).
stays visible. The open issues that users can hit are listed in the Known Issues
sections of the [Parquet](../../user-guide/latest/parquet.md#known-issues) and
[Iceberg](../../user-guide/latest/iceberg.md#known-issues) guides.

```{toctree}
:maxdepth: 1
Expand Down
4 changes: 2 additions & 2 deletions docs/source/contributor-guide/data-source-audits/parquet.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

> Audit notes for the native Parquet scan and the native Parquet writer. Absence of an entry means
> the area has not been audited yet. Open issues are listed for users in
> [Known Issues in Parquet and Iceberg Reads and Writes].
> [Known Issues in the Parquet guide].

## Native Parquet scan

Expand Down Expand Up @@ -87,7 +87,7 @@
not support ([#3193], [#3194]), object-store and HDFS outputs, and write throughput, which needs a
release build.

[Known Issues in Parquet and Iceberg Reads and Writes]: ../../user-guide/latest/compatibility/parquet-iceberg-known-issues.md
[Known Issues in the Parquet guide]: ../../user-guide/latest/parquet.md#known-issues
[apache/datafusion#26393]: https://github.com/apache/datafusion/issues/26393
[#3193]: https://github.com/apache/datafusion-comet/issues/3193
[#3194]: https://github.com/apache/datafusion-comet/issues/3194
Expand Down
2 changes: 1 addition & 1 deletion docs/source/contributor-guide/iceberg-writes.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ the JVM/native boundary in each direction, who owns cleanup when a task fails, a
change to any of it.

For the user-facing view (configuration, the eligibility table, and the accepted differences from
iceberg-java's output), see [Iceberg Writes](../user-guide/latest/iceberg-writes.md). This page
iceberg-java's output), see [Iceberg Writes](../user-guide/latest/iceberg.md#writes). This page
does not repeat those lists; it explains the code that implements them.

## Overview
Expand Down
2 changes: 1 addition & 1 deletion docs/source/contributor-guide/timezones.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ aimed at contributors working on datetime expressions, casts, scans, or any nati
produces or consumes timestamp columns. For user-facing differences from Spark, see the
datetime and cast sections of the
[Expression Compatibility](../user-guide/latest/compatibility/expressions/index.md) pages, and the
[scan compatibility](../user-guide/latest/compatibility/scans.md) notes. Known bugs are tracked in
[Parquet](../user-guide/latest/parquet.md) notes. Known bugs are tracked in
[#6335](https://github.com/apache/datafusion-comet/issues/6335).

The short version: Comet never converts timestamp values at the JVM/native boundary. The session
Expand Down
10 changes: 5 additions & 5 deletions docs/source/user-guide/latest/compatibility/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ Comet aims to provide consistent results with the version of Apache Spark that i

This guide documents areas where Comet's behavior is known to differ from Spark. Topics are grouped by subsystem:

- **Parquet**: limitations when reading Parquet files.
- **Parquet and Iceberg known issues**: open issues in which Parquet and Iceberg reads and writes
give different results from Spark, or fail where Spark succeeds, without falling back.
- **Parquet and Iceberg**: limitations, differences from Spark, and known issues for reading and
writing Parquet files and Iceberg tables. These live with the data source, on the
[Parquet](../parquet.md) and [Iceberg](../iceberg.md) pages.
- **Floating-point comparison**: NaN and signed-zero handling in comparisons.
- **Regular expressions**: differences between the Rust regexp crate and Java's regex engine.
- **Operators**: operator-level compatibility notes, including window functions and round-robin partitioning.
Expand Down Expand Up @@ -122,8 +122,8 @@ divergence:
The following native paths silently return values that differ from Spark for edge-case inputs.
Most also have entries in the per-category expression pages linked above; they are collected here
so users hunting an unexpected value have a single place to check. Parquet and Iceberg reads and
writes have their own list, in
[Known Issues in Parquet and Iceberg Reads and Writes](parquet-iceberg-known-issues.md#results-that-differ-from-spark):
writes have their own lists, under Known Issues on the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This tells readers that the Parquet and Iceberg issues are under Known Issues on those two pages, but the object-store and credential entries moved to the S3 and Azure Limitations lists in datasources.md. Moving them makes sense, since they depend on storage rather than format. But someone checking Known Issues for a read that fails where Spark succeeds won't find them now. A sentence in each Known Issues section would close the gap, for example: "Storage-specific issues are listed under the S3 and Azure limitations."

Those two lists sit under headings that are both ### Limitations, so their anchors come out as #limitations and #limitations-1, and the second one changes meaning if another Limitations heading appears earlier. Renaming them to ### S3 Limitations and ### Azure Limitations would give stable anchors. Nothing links to #limitations today.

[Parquet](../parquet.md#known-issues) and [Iceberg](../iceberg.md#known-issues) pages:

- Native `RANGE` window frames with an explicit `PRECEDING` / `FOLLOWING` offset diverge from
Spark when the boundary arithmetic overflows for `DATE` or `DECIMAL` `ORDER BY` columns
Expand Down
Loading