[
https://issues.apache.org/jira/browse/HCATALOG-241?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13204655#comment-13204655
]
[email protected] commented on HCATALOG-241:
--------------------------------------------------------
bq. On 2012-02-09 01:56:49, Alan Gates wrote:
bq. > /trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java, line 33
bq. > <https://reviews.apache.org/r/3819/diff/1/?file=73309#file73309line33>
bq. >
bq. > In the design notes posted on HCATALOG-237 we had discussed not
caching deserialized values. Doing so increases your memory usage for likely
no reason (usually records are only read once).
True, and I did start out that way, but LazyHCatRecord extends HCatRecord which
implements WritableComparable - this means the following:
a) List<Object> .getAll() is something we need to support - and this is
something that can be called by layers above us, which means we do need a fully
deserialized version if requested anyway.
b) For .compareTo() to work, same deal.
I can take it out and make it only locally deserialize the entire object when
needed.
bq. On 2012-02-09 01:56:49, Alan Gates wrote:
bq. > /trunk/src/java/org/apache/hcatalog/data/HCatRecordSerDe.java, line 197
bq. > <https://reviews.apache.org/r/3819/diff/1/?file=73307#file73307line197>
bq. >
bq. > Why did this move from private to public static? Seems like it's
only used by LazyHCatRecord, so it could at least be package private.
I changed it to static because I saw no reason for it to be an object method,
and changed it to public because I felt it could be used as a utility function
- I had initially wrapped it and called it from HCatUtil before changing that
to just go from LazyHCatRecord itself. Now, there's no reason for it, and I can
change it back.
bq. On 2012-02-09 01:56:49, Alan Gates wrote:
bq. > /trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspector.java,
line 51
bq. > <https://reviews.apache.org/r/3819/diff/1/?file=73306#file73306line51>
bq. >
bq. > This is a significant behavioral change, to now return null rather
than crash when we can't decode a field. It may be ok, but are we sure? Also,
the log message here should be at least a warn, not a debug which will usually
get ignored.
This one is something I had misgivings about, and I'm still a bit on-the-edge.
The method we override does not allow throwing an exception, so we aren't
allowed to throw an exception. Earlier, the get method we depended on
wouldn't/couldn't throw an exception. Earlier, we had the objects at hand, and
were only retrieving it. Now, the underlying get can.
The case where it manifests itself is if the underlying object has malformed
data that causes a deserialization error, and we choose to ignore that and
present nulls to the layers above. I'm not sure I like the hiding behaviour
myself.
- Sushanth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3819/#review4951
-----------------------------------------------------------
On 2012-02-09 00:54:29, Sushanth Sowmyan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/3819/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-02-09 00:54:29)
bq.
bq.
bq. Review request for Alan Gates and Francis Liu.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Initial upload.
bq.
bq.
bq. This addresses bug HCATALOG-241.
bq. https://issues.apache.org/jira/browse/HCATALOG-241
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1242037
bq. /trunk/src/java/org/apache/hcatalog/data/DefaultHCatRecord.java 1242037
bq. /trunk/src/java/org/apache/hcatalog/data/HCatRecord.java 1242037
bq. /trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspector.java
1242037
bq. /trunk/src/java/org/apache/hcatalog/data/HCatRecordSerDe.java 1242037
bq. /trunk/src/java/org/apache/hcatalog/data/HCatRecordable.java 1242037
bq. /trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java
PRE-CREATION
bq. /trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java
1242037
bq.
bq. Diff: https://reviews.apache.org/r/3819/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Sushanth
bq.
bq.
> Changes to HCatRecord to support switch from StorageDriver to SerDe
> -------------------------------------------------------------------
>
> Key: HCATALOG-241
> URL: https://issues.apache.org/jira/browse/HCATALOG-241
> Project: HCatalog
> Issue Type: Sub-task
> Affects Versions: 0.4
> Reporter: Alan Gates
> Assignee: Sushanth Sowmyan
> Fix For: 0.4
>
> Attachments: HCATALOG-241.patch
>
>
> This JIRA tracks changes to HCatRecord. See HCATALOG-237 for details and
> design notes.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
