Skip to content

update int64 columns to use string values#31

Merged
wesm merged 3 commits intoapache:masterfrom
emkornfield:int64_to_string
Jun 29, 2020
Merged

update int64 columns to use string values#31
wesm merged 3 commits intoapache:masterfrom
emkornfield:int64_to_string

Conversation

@emkornfield
Copy link
Copy Markdown
Contributor

@wesm I hand edited (with vim macro) and then verified it can be parsed with python (I didn't rerun integration tests. Is there an easy way to do that? Also it looks like the only int64 usage I could find is with primitives (the file edited and zero batch test cases).

@wesm
Copy link
Copy Markdown
Member

wesm commented Jun 29, 2020

It's possible this dictionary test uses strings too https://github.com/apache/arrow/blob/master/dev/archery/archery/integration/datagen.py#L1426

@emkornfield
Copy link
Copy Markdown
Contributor Author

emkornfield commented Jun 29, 2020

Thanks updated the dictionary.json.gz as well.

@pitrou
Copy link
Copy Markdown
Member

pitrou commented Jun 29, 2020

Can you explain the changes done?

@emkornfield
Copy link
Copy Markdown
Contributor Author

Changes:

  1. Gunzip affected file
  2. Find values related to int64/uint64 columns and put them in quotes.
  3. Gzip the files

@pitrou
Copy link
Copy Markdown
Member

pitrou commented Jun 29, 2020

I thanks. I hadn't noticed those were the JSON files.

Copy link
Copy Markdown
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

+1

@wesm
Copy link
Copy Markdown
Member

wesm commented Jun 29, 2020

@emkornfield @pitrou I'll try to test these locally

@wesm
Copy link
Copy Markdown
Member

wesm commented Jun 29, 2020

Ah the interval and datetime tests also have the issue

https://gist.github.com/wesm/bf650d27557e90b4833054ab01783623

I'll try to fix them locally

@wesm
Copy link
Copy Markdown
Member

wesm commented Jun 29, 2020

OK, passing now (fun times with emacs regexp-replace).

https://gist.github.com/wesm/b7ed6934ef7753c2087919156c751bf4

+1

@wesm wesm merged commit bb555cd into apache:master Jun 29, 2020
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.

3 participants