docs: say what a new operator owes beyond its own code - #8459
Open
kz930 wants to merge 7 commits into
Open
Conversation
27 tasks
github-actions
Bot
added
docs
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8459 +/- ##
============================================
- Coverage 94.11% 93.99% -0.13%
- Complexity 4811 4862 +51
============================================
Files 1197 1204 +7
Lines 48813 49071 +258
Branches 5906 5945 +39
============================================
+ Hits 45939 46122 +183
- Misses 1420 1475 +55
- Partials 1454 1474 +20
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The Java and Python guides cover writing the operator. What they do not cover is everything a new one has to satisfy afterwards: a stated constraint on each config property, the standalone Python fragment the exporter needs, a green verification run, and where to record a run that cannot happen. Written from the code rather than from memory, so it names the mechanisms as they are: bounds come from a field's JsonSchemaInject rather than from Min and Max, which nothing here uses; a pattern is read by the browser and by the verification generator, and only one of the two matches whole strings. Two diagrams carry the shape of the translation and of the two compared paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kz930
force-pushed
the
docs/guide-to-add-a-new-operator
branch
from
September 9, 2026 05:19
0c9255b to
334af61
Compare
The page said to implement `getOutputSchemas`. That is the Python operator's way; a Java or Scala one declares its output through a `SchemaPropagationFunc` inside `getPhysicalOp`, which is what 32 of the registered operators do. And escaping has a second half. A `pyb` field has to reach the template whole, so joining it to anything in Scala first defeats it — the protected value is handed over as a plain string, and the placeholder leaks into the emitted Python. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The page told a new operator to implement `generateStandaloneCode()` and said nothing about the declared type, which is what decides how a column becomes text. Adding the boolean column to the canonical table also made the count of hostile-named columns wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI builds its interpreter from the requirement files on every run, so a venv left behind locally tests library versions the product never sees, and a failure only there reads like a defect in the code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@carloea2 this one is documentation: what a new operator owes beyond its own code, written from what the last few operators actually needed. Would you take a look when you have a moment? |
The guide described the empty-cell run and stopped there. The empty-table run asks a different question: that table still has a value in every column somewhere, so code reading a range or a quantile finds one, while this one has nothing to read. The venv line named one requirements file. An operator's libraries are in the other. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guide covered the canonical table and said only that source operators bring their own file. A contributor adding one could not tell whether SourceCategoryRunner needed editing, and for the common case it does not: a scan source whose declared format already has an encoder is verified with no code there at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Writing the Parquet source cost a debugging cycle on this: the file counts from the epoch, a Texera TIMESTAMP is a wall clock, and reading the count with `new Timestamp(millis)` moves it by the machine's own offset while pandas reads the same number as UTC. The two paths then part by that offset, and agree again on a machine set to UTC, so a CI that runs there never shows it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
What changes were proposed in this PR?
A page for contributors adding an operator, covering what the existing Java and
Python guides do not: declaring the output schema, stating a constraint on every
config property, writing the standalone Python fragment the exporter needs,
getting a green verification run, and recording a run that cannot happen.
Two diagrams carry the shape of the translation and of the two compared paths.
AGENTS.mdgains a line on the same footing: the Python venv has to berebuilt from the requirement files when they change, since CI builds its
interpreter from them on every run and a stale one tests versions the product
never sees.
Each rule here was read out of the code rather than recalled, which corrected
three things a reader would otherwise have been told wrong. Numeric bounds come
from a field's
@JsonSchemaInject, not from@Minand@Max, which nothing inthe repository uses and whose upper bound the configuration generator never
reads. A
patternis read both by the browser and by that generator, and onlythe second matches whole strings, so an unanchored one does not mean the same
thing to both. And
WCS_TEST_FILTERis how CI splits its two jobs, notsomething a local run sets.
Any related issues, documentation, discussions?
Part of #8325, 27 of 27; that issue lists the set in order.
Closes #8458, the task this change is the whole of.
The two guides this one sits beside are
guide-to-implement-java-operatorandguide-to-implement-python-operator;they cover the operator's own logic, and stop there.
getOutputSchemasappearsin neither, although every operator must implement it, and a schema that
disagrees with what the executor emits fails at run time rather than at compile
time.
How was this PR tested?
Documentation only, so there is nothing to run. Every claim in it was checked
against the code it describes rather than against memory: the annotation names
against their declarations, the fill rules against
ConfigGenerator, theenvironment variables against
build.sbtand the CI workflow, and the twoimage references and two internal links against the files they name.
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)