Skip to content

MINOR: Fix VariantUtil.getByte() Javadoc copied from getShort()#3649

Open
anxkhn wants to merge 1 commit into
apache:masterfrom
anxkhn:patch-2
Open

MINOR: Fix VariantUtil.getByte() Javadoc copied from getShort()#3649
anxkhn wants to merge 1 commit into
apache:masterfrom
anxkhn:patch-2

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 7, 2026

Copy link
Copy Markdown

Rationale for this change

The Javadoc on the package-private helper VariantUtil.getByte(ByteBuffer) is a
verbatim copy of the getShort(ByteBuffer) block directly above it, and was never
adjusted for getByte. It currently reads:

/**
 * Similar to getLong(), but for the types: Type.BYTE, SHORT.
 * @param value The Variant value
 * @return The short value
 */
static byte getByte(ByteBuffer value) {

Both lines contradict the method:

  • getByte does not handle Type.SHORT. Its switch has only case INT8: (the
    BYTE type); every other type, SHORT included, falls through to
    throw unexpectedType(Variant.Type.BYTE, value).
  • @return The short value contradicts the declared static byte getByte(...)
    return type; the method returns (byte) readLong(value, value.position() + 1, 1).

getShort above it genuinely handles both INT8 and INT16 and returns short,
so the doc block was clearly copy-pasted onto getByte and left stale.

What changes are included in this PR?

A two-line Javadoc correction on getByte (in
parquet-variant/src/main/java/org/apache/parquet/variant/VariantUtil.java):

   /**
-   * Similar to getLong(), but for the types: Type.BYTE, SHORT.
+   * Similar to getLong(), but for the type: Type.BYTE.
    * @param value The Variant value
-   * @return The short value
+   * @return The byte value
    */
   static byte getByte(ByteBuffer value) {

The (correct) getShort Javadoc directly above is left untouched. No code behavior
changes.

Are these changes tested?

No new test. This is a Javadoc correction with no runtime behavior change, so a
unit test would not assert anything meaningful. The corrected wording simply makes
the internal contract match the already-tested behavior (TestVariantScalar
exercises getByte on BYTE values and the invalid-read paths). I confirmed the
module still passes its gates locally:

  • mvn -pl parquet-variant spotless:check -> BUILD SUCCESS (palantir-java-format
    gate passes)
  • mvn -pl parquet-variant -am -Dspotless.check.skip=true test -> BUILD SUCCESS
    (parquet-variant: 180 run, 0 failures, 0 errors, 1 skipped)

Are there any user-facing changes?

No. Documentation (Javadoc) only, on a package-private helper. No API or behavior
change.

The Javadoc on the package-private VariantUtil.getByte() was copied
verbatim from getShort() and never adjusted. It listed SHORT as a
supported type and documented an @return of "short value", but the
method only accepts the BYTE/INT8 type and returns a byte.

Correct the doc to describe Type.BYTE and an @return of "The byte
value". No behavioral change.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant