[
https://issues.apache.org/jira/browse/PHOENIX-2120?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14634257#comment-14634257
]
Samarth Jain commented on PHOENIX-2120:
---------------------------------------
In PTableImpl.java, the method rowKeyOrderOptimizable() should be called
instead.
{code}
if (maxLength != null && type.isFixedWidth() && byteValue.length < maxLength) {
+ if (rowKeyOrderOptimizable) {
+ key.set(byteValue);
+ type.pad(key, maxLength, sortOrder);
+ byteValue = ByteUtil.copyKeyBytesIfNecessary(key);
+ }
{code}
In UpgradeUtil#upgradeDescVarLengthRowKeys, is it possible for the connection
to have scn and tenant id properties set? I see that the only caller of this
method is PhoenixRuntime and there we are getting the connection using:
{code}
Properties props = new Properties();
conn = DriverManager.getConnection(jdbcUrl, props)
.unwrap(PhoenixConnection.class);
{code}
{code}
public static void upgradeDescVarLengthRowKeys(PhoenixConnection conn,
List<String> tablesToUpgrade) throws SQLException {
if (conn.getClientInfo(PhoenixRuntime.CURRENT_SCN_ATTRIB) != null) {
throw new SQLException("May not specify the CURRENT_SCN property
when upgrading");
}
if (conn.getClientInfo(PhoenixRuntime.TENANT_ID_ATTRIB) != null) {
throw new SQLException("May not specify the TENANT_ID_ATTRIB
property when upgrading");
}
{code}
Extremely minor nits:
remove TODO from @param bypassUpgrade TODO in
{code}
private static boolean upgradeSharedIndex
public static void upgradeDescVarLengthRowKeys
{code}
Should the second comment be Upgrade local indexes?
{code}
// Upgrade view indexes
- wasUpgraded |= upgradeSharedIndex(upgradeConn, conn,
sharedViewIndexName);
+ wasUpgraded |= upgradeSharedIndex(upgradeConn, conn,
sharedViewIndexName, bypassUpgrade);
String sharedLocalIndexName =
Bytes.toString(MetaDataUtil.getLocalIndexPhysicalName(table.getName().getBytes()));
// Upgrade view indexes
- wasUpgraded |= upgradeSharedIndex(upgradeConn, conn,
sharedLocalIndexName);
+ wasUpgraded |= upgradeSharedIndex(upgradeConn, conn,
sharedLocalIndexName, bypassUpgrade);
{code}
A general note - considering this upgrade is user driven, it would be good to
include the information in release notes along with examples. Especially for
this case where the trailing spaces could possibly be legitimate:
{code}
else if (field.getDataType() == PBinary.INSTANCE) {
+ // Remove trailing space characters so
that the setValues call below will replace them
+ // with the correct zero byte
character. Note this is somewhat dangerous as these
+ // could be legit, but I don't know
what the alternative is.
+ int len = ptr.getLength();
+ while (len > 0 &&
ptr.get()[ptr.getOffset() + len - 1] == StringUtil.SPACE_UTF8) {
+ len--;
+ }
+ ptr.set(ptr.get(), ptr.getOffset(),
len);
}
{code}
> Padding character is not inverted as required for DESC CHAR columns
> -------------------------------------------------------------------
>
> Key: PHOENIX-2120
> URL: https://issues.apache.org/jira/browse/PHOENIX-2120
> Project: Phoenix
> Issue Type: Improvement
> Reporter: James Taylor
> Attachments: PHOENIX-2120_v1.patch, PHOENIX-2120_v2.patch
>
>
> Looks like the padding character (a space char) is not inverted as it should
> be for DESC CHAR columns. This can lead to rows not being in the correct
> order (similar to PHOENIX-2067). If an application does not rely on auto
> padding, they will not be affected.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
