Skip to content

[SPARK-10392] [SQL] Pyspark - Wrong DateType support on JDBC connection #8556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

[SPARK-10392] [SQL] Pyspark - Wrong DateType support on JDBC connection #8556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7985de7
[SPARK-10392] [SQL] Pyspark - Wrong DateType support on JDBC connection
0x0FFF Sep 1, 2015
f41b125
[SPARK-10392] [SQL] Regression test added
0x0FFF Sep 1, 2015
a7fd681
[SPARK-10392] [SQL] Moving regression test to DataTypeTests class
0x0FFF Sep 1, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions python/pyspark/sql/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@ def test_decimal_type(self):
t3 = DecimalType(8)
self.assertNotEqual(t2, t3)

# regression test for SPARK-10392
def test_datetype_equal_zero(self):
dt = DateType()
self.assertEqual(dt.fromInternal(0), datetime.date(1970, 1, 1))


class SQLTests(ReusedPySparkTestCase):

Expand Down
6 changes: 4 additions & 2 deletions python/pyspark/sql/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,12 @@ def needConversion(self):
return True

def toInternal(self, d):
return d and d.toordinal() - self.EPOCH_ORDINAL
if d is not None:
Copy link
Contributor

@davies davies Sep 1, 2015

Choose a reason for hiding this comment

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

I think this change is not needed, d will always be true if it's not None

Copy link
Contributor Author

@0x0FFF 0x0FFF Sep 1, 2015

Choose a reason for hiding this comment

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

It will be false for None, empty string, empty list, 0. This way it will be returned instead of the date:

>>> [] and 'foo'
[]
>>> 0 and 'foo'
0
>>> 1 and 'foo'
'foo'

Copy link
Contributor

@davies davies Sep 1, 2015

Choose a reason for hiding this comment

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

d could not be list/int/string, it can only be Date or None.

Copy link
Contributor Author

@0x0FFF 0x0FFF Sep 1, 2015

Choose a reason for hiding this comment

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

Sorry, I see. Should I revert this change? It is mostly for both methods to follow the same logic, as return x and y is not a very readable code in my opinion. Also the class TimestampType is implemented in a similar way - both methods are starting with input parameter check for None

Copy link
Contributor

@davies davies Sep 1, 2015

Choose a reason for hiding this comment

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

Either is good to me.

return d.toordinal() - self.EPOCH_ORDINAL

def fromInternal(self, v):
return v and datetime.date.fromordinal(v + self.EPOCH_ORDINAL)
if v is not None:
return datetime.date.fromordinal(v + self.EPOCH_ORDINAL)


class TimestampType(AtomicType):
Expand Down