Skip to content

Conversation

@smungee
Copy link
Contributor

@smungee smungee commented May 13, 2016

What changes were proposed in this pull request?

Fix MapObjects.itemAccessorMethod to handle TimestampType. Without this fix, Array[Timestamp] cannot be properly encoded or decoded. To reproduce this, in ExpressionEncoderSuite, if you add the following test case:

encodeDecodeTest(Array(Timestamp.valueOf("2016-01-29 10:00:00")), "array of timestamp")
... you will see that (without this fix) it fails with the following output:

- encode/decode for array of timestamp: [Ljava.sql.Timestamp;@fd9ebde *** FAILED ***
  Exception thrown while decoding
  Converted: [0,1000000010,800000001,52a7ccdc36800]
  Schema: value#61615
  root
  -- value: array (nullable = true)
      |-- element: timestamp (containsNull = true)
  Encoder:
  class[value[0]: array<timestamp>] (ExpressionEncoderSuite.scala:312)

How was this patch tested?

Existing tests

@HyukjinKwon
Copy link
Member

(I think it might needs a JIRA because it seems changing existing behaviour)

@smungee
Copy link
Contributor Author

smungee commented May 14, 2016

@HyukjinKwon I think the original author might have just forgotten to add the TimestampType to the match..

@smungee
Copy link
Contributor Author

smungee commented May 14, 2016

The bug is quite easy to reproduce. In ExpressionEncoderSuite, if you add the following test case:

encodeDecodeTest(Array(Timestamp.valueOf("2016-01-29 10:00:00")), "array of timestamp")

... you will see that (without this fix) it fails with the following output:

- encode/decode for array of timestamp: [Ljava.sql.Timestamp;@fd9ebde *** FAILED ***
  Exception thrown while decoding
  Converted: [0,1000000010,800000001,52a7ccdc36800]
  Schema: value#61615
  root
  -- value: array (nullable = true)
      |-- element: timestamp (containsNull = true)


  Encoder:
  class[value[0]: array<timestamp>] (ExpressionEncoderSuite.scala:312)

@smungee smungee changed the title Fix MapObjects.itemAccessorMethod to handle TimestampType Fix bug where Array[Timestamp] cannot be encoded/decoded correctly May 14, 2016
@techaddict
Copy link
Contributor

@smungee Can you please add tests for this ?

@smungee
Copy link
Contributor Author

smungee commented May 14, 2016

@techaddict Added a test

@smungee smungee changed the title Fix bug where Array[Timestamp] cannot be encoded/decoded correctly [SPARK-15321] Fix bug where Array[Timestamp] cannot be encoded/decoded correctly May 14, 2016
@rxin
Copy link
Contributor

rxin commented May 20, 2016

@smungee sorry to have this gone stale. Do you mind bringing it up to date? This seems useful to fix.

@smungee smungee force-pushed the fix-itemAccessorMethod branch from 387e6c9 to b2f2914 Compare May 20, 2016 02:09
@smungee
Copy link
Contributor Author

smungee commented May 20, 2016

@rxin The code in question seems to have been removed in this commit by @cloud-fan

c36ca65

With the latest SNAPSHOT version, I no longer see the bug. I've updated this PR to include just my additional test-case, just to make sure it doesn't happen again. I'd recommend adding this test-case in.

@SparkQA
Copy link

SparkQA commented May 20, 2016

Test build #2999 has finished for PR 13108 at commit 387e6c9.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

LGTM, merging to master and 2.0, thanks!

@asfgit asfgit closed this in d5c47f8 May 20, 2016
asfgit pushed a commit that referenced this pull request May 20, 2016
…d correctly

## What changes were proposed in this pull request?

Fix `MapObjects.itemAccessorMethod` to handle `TimestampType`. Without this fix, `Array[Timestamp]` cannot be properly encoded or decoded. To reproduce this, in `ExpressionEncoderSuite`, if you add the following test case:

`encodeDecodeTest(Array(Timestamp.valueOf("2016-01-29 10:00:00")), "array of timestamp")
`
... you will see that (without this fix) it fails with the following output:

```
- encode/decode for array of timestamp: [Ljava.sql.Timestamp;fd9ebde *** FAILED ***
  Exception thrown while decoding
  Converted: [0,1000000010,800000001,52a7ccdc36800]
  Schema: value#61615
  root
  -- value: array (nullable = true)
      |-- element: timestamp (containsNull = true)
  Encoder:
  class[value[0]: array<timestamp>] (ExpressionEncoderSuite.scala:312)
```

## How was this patch tested?

Existing tests

Author: Sumedh Mungee <[email protected]>

Closes #13108 from smungee/fix-itemAccessorMethod.

(cherry picked from commit d5c47f8)
Signed-off-by: Wenchen Fan <[email protected]>
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.

6 participants