Skip to content

adding support for complex keys#728

Merged
vinothchandar merged 9 commits intoapache:masterfrom
jaimin-shah:master
Jun 21, 2019
Merged

adding support for complex keys#728
vinothchandar merged 9 commits intoapache:masterfrom
jaimin-shah:master

Conversation

@jaimin-shah
Copy link
Contributor

Resolving the issue related to ambiguity in recordKey by creating and parsing json object as string.

Now HoodieKey looks like this:
HoodieKey { recordKey={"_row_key":"16bf0b32-7557-42ac-b367-9fe32ae4795e","timestamp":"0.0"} partitionPath=rider-002/driver-002}

Jaimin Shah added 2 commits June 12, 2019 14:57
Resolving the issue related to ambiguity in recordKey by creating and parsing json object as string.
@jaimin-shah
Copy link
Contributor Author

jaimin-shah commented Jun 12, 2019

HI I think scope for GSON is limited to test we might need to change scope to compile to make this work.

@jaimin-shah jaimin-shah reopened this Jun 14, 2019
@vinothchandar
Copy link
Member

Hi @jaimin-shah Will take a look at this sometime over hte weekend. Been having some travis CI issues lately, that is keeping few of us pretty busy .. :(

Copy link
Member

@vinothchandar vinothchandar left a comment

Choose a reason for hiding this comment

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

can you please add an small unit test?

}

JsonObject recordKeyJson = new JsonObject();

Copy link
Member

Choose a reason for hiding this comment

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

nit: remove extra line

recordKeyJson.addProperty(recordKeyField,DataSourceUtils.getNestedFieldValAsString(record, recordKeyField));
}
Gson gson = new Gson();
String recordKey = gson.toJson(recordKeyJson);
Copy link
Member

Choose a reason for hiding this comment

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

instead of json, can we just concatenate the recordKeyFields? This adds additional json building, parsing in the fast path. any specific reasons you choose json for the recordKey?

}
partitionPath.delete(partitionPath.length() - 1, partitionPath.length());
} catch (HoodieException e) {
// TODO : optimize this since throwing and catching exception is cpu intensive
Copy link
Member

Choose a reason for hiding this comment

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

but is that a common scenario? its probably ok to do this when misconfigured etc right? if you agree, we can remove this TODO

partitionPath.append(DataSourceUtils.getNestedFieldValAsString(record, partitionPathField));
partitionPath.append(DEFAULT_PARTITION_PATH_SEPARATOR);
}
partitionPath.delete(partitionPath.length() - 1, partitionPath.length());
Copy link
Member

Choose a reason for hiding this comment

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

use StringBuilder.html#deleteCharAt(int) ?

Jaimin Shah added 3 commits June 18, 2019 14:44
row_key looks like this now
row_key:16bf0b32-7557-42ac-b367-9fe32ae4795e,timestamp:0.0

Row_Key generated by concatanation instead of JSON.
@vinothchandar vinothchandar merged commit 17e878f into apache:master Jun 21, 2019
bvaradar pushed a commit to bvaradar/hudi that referenced this pull request Sep 19, 2019
- Resolving the issue related to ambiguity in recordKey by creating and parsing json object as string.
- added unit test for ComplexKeyGenerator
- minor changes
vinishjail97 pushed a commit to vinishjail97/hudi that referenced this pull request Jun 5, 2024
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.

2 participants