ORC-642: update PatchedBase doc with patch ceiling in spec

[Jefffrey]

What changes were proposed in this pull request?

Update PatchedBase specification doc to include details about the behaviour of padding the patch gap + patch width bits to nearest fixed btis.

Why are the changes needed?

Ensure spec is accurate to implementation details

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

No

[Jefffrey]

From

orc/c++/src/RLEV2Util.cc

Lines 29 to 33 in 9b79de9

	// Map bit length i to closest fixed bit width that can contain i bits.
	const uint8_t ClosestFixedBitsMap[65] = {
	1, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21,
	22, 23, 24, 26, 26, 28, 28, 30, 30, 32, 32, 40, 40, 40, 40, 40, 40, 40, 40, 48, 48, 48,
	48, 48, 48, 48, 48, 56, 56, 56, 56, 56, 56, 56, 56, 64, 64, 64, 64, 64, 64, 64, 64};

[wgtmac]

Is it better to follow the java code to add 0 as well?

orc/java/core/src/java/org/apache/orc/impl/SerializationUtils.java

Lines 362 to 391 in 513922a

	public int getClosestFixedBits(int n) {
	if (n == 0) {
	return 1;
	}
	if (n <= 24) {
	return n;
	}
	if (n <= 26) {
	return 26;
	}
	if (n <= 28) {
	return 28;
	}
	if (n <= 30) {
	return 30;
	}
	if (n <= 32) {
	return 32;
	}
	if (n <= 40) {
	return 40;
	}
	if (n <= 48) {
	return 48;
	}
	if (n <= 56) {
	return 56;
	}
	return 64;
	}

[Jefffrey]

I don't think it makes sense to have 0 as the input is PGW + PW, and the spec states that PGW is 1 to 8 bits and PW is 1 to 64 bits, so it can never be 0 (or 1, for that matter, I guess 🤔 )

[Jefffrey]

According to

orc/c++/src/RleDecoderV2.cc

Line 332 in 9b79de9

uint32_t cfb = getClosestFixedBits(patchBitSize + pgw);

[wgtmac]

Could you please rename ceil to closestFixedBits or cfb? ceil does not seems to have its common meaning here.

[Jefffrey]

Sure thing, done

[wgtmac]

Thanks for the fix! Could you please also add them to ORCv2.md as well?

[wgtmac]

Could you please rename ceil to closestFixedBits or cfb? ceil does not seems to have its common meaning here.

[wgtmac]

Is it better to follow the java code to add 0 as well?

orc/java/core/src/java/org/apache/orc/impl/SerializationUtils.java

Lines 362 to 391 in 513922a

	public int getClosestFixedBits(int n) {
	if (n == 0) {
	return 1;
	}
	if (n <= 24) {
	return n;
	}
	if (n <= 26) {
	return 26;
	}
	if (n <= 28) {
	return 28;
	}
	if (n <= 30) {
	return 30;
	}
	if (n <= 32) {
	return 32;
	}
	if (n <= 40) {
	return 40;
	}
	if (n <= 48) {
	return 48;
	}
	if (n <= 56) {
	return 56;
	}
	return 64;
	}

[wgtmac]

LGTM +1

[wgtmac]

cc @dongjoon-hyun @deshanxiao

[dongjoon-hyun]

+1, LGTM.

[deshanxiao]

Thanks @Jefffrey @wgtmac @dongjoon-hyun . Merged to main.