[
https://issues.apache.org/jira/browse/HCATALOG-241?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13204960#comment-13204960
]
[email protected] commented on HCATALOG-241:
--------------------------------------------------------
bq. On 2012-02-09 19:26:50, Francis Liu wrote:
bq. > /trunk/src/java/org/apache/hcatalog/data/HCatRecordable.java, line 36
bq. > <https://reviews.apache.org/r/3819/diff/1/?file=73308#file73308line36>
bq. >
bq. > it would be nice if we didn't have to throw checked exception but
runtime exceptions instead for these methods. I have no idea what the user
would get out of catching these exceptions.
Agreed. As per your note above on RuntimeException, I can change it here, and
that ripples through the rest of it.
bq. On 2012-02-09 19:26:50, Francis Liu wrote:
bq. > /trunk/src/java/org/apache/hcatalog/data/HCatRecordSerDe.java, line 136
bq. > <https://reviews.apache.org/r/3819/diff/1/?file=73307#file73307line136>
bq. >
bq. > I'm a bit lost here the comments and implementation seem to be meant
for the previous experimental implementnation? Maybe we can clean some of these
up?
Well, yes, HCatRecordSerDe is mostly unchanged from HCATALOG-204 with the
exception of one additional initialize() method that takes a HCatSchema
parameter, making it easier to call from the HCat world.
The one additional change I should make to this, looking at it again, now, is
to replace the serialize() method with a ctor to LazyHCatRecord rather than to
DefaultHCatRecord (although, if we're removing .getAll, etc, then it should
probably remain DefaultHCatRecord)
bq. On 2012-02-09 19:26:50, Francis Liu wrote:
bq. > /trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspector.java,
line 47
bq. > <https://reviews.apache.org/r/3819/diff/1/?file=73306#file73306line47>
bq. >
bq. > If memory serves assertions are disabled at runtime by default since
it's supposed to be a testing/debugging tool. This check seems good enough to
be an if condition?
Agreed, changing.
bq. On 2012-02-09 19:26:50, Francis Liu wrote:
bq. > /trunk/src/java/org/apache/hcatalog/data/HCatRecord.java, line 148
bq. > <https://reviews.apache.org/r/3819/diff/1/?file=73305#file73305line148>
bq. >
bq. > I would argue that we shouldn't be implementing this since we
incomplete knowledge of the users's data. Any comparisons needed internally
should be done via a comparator. Same goes with compareTo
I'm still iffy with this. Without a compareTo and an equals, HCatRecord isn't a
WritableComparable. HCatInputFormat extends InputFormat<? extends
WritableComparable,? extends WritableComparable>. I know that the key needs to
be WritableComparable, but am unsure about the value. Let me dig in a bit, and
if we don't need it to extend Comparable, then we can go ahead and change this
- this will be a backward incompatible change though, since we have been
exposing HCatRecord to users, and it has thus far had this ability.
bq. On 2012-02-09 19:26:50, Francis Liu wrote:
bq. > /trunk/src/java/org/apache/hcatalog/data/HCatRecord.java, line 143
bq. > <https://reviews.apache.org/r/3819/diff/1/?file=73305#file73305line143>
bq. >
bq. > you be comparing this. you're breaking the transitivity contract for
the compareTo method
bq.
bq. Francis Liu wrote:
bq. *you shouldn't be comparing this.
bq.
bq. Francis Liu wrote:
bq. oops i meant reflexive.
I'm not sure I understand - compareTo returns -1, 0 or 1 depending on the
comparison.
a.compareTo(b) is intended to be negative of b.compareTo(a) unless they are
equal. A compare operation is not reflexive.
bq. On 2012-02-09 19:26:50, Francis Liu 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. > shouldn't this be called deserializeField?
Technically, for hive, no. :)
Hive sits on top of the SerDe, and is using this SerDe to "serialize" an
object/objectinspector it provides to a Writable representation, which in this
case, is HCatRecord. So the process of converting an object-objectinspector
pair into HCatRecord, from hive's perspective, is serialization.
The terminology is a bit confusing depending on which layer we see it from, but
I've tried to keep it consistent with the other serdes, and name it as hive
would see it.
- Sushanth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3819/#review4977
-----------------------------------------------------------
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
