Skip to content
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

Jump to bottom

[JS] Implement Duration type, integration test support for Interval and Duration types #21815

Closed
asfimport opened this issue May 16, 2019 · 4 comments · Fixed by #37341
Closed

[JS] Implement Duration type, integration test support for Interval and Duration types #21815

asfimport opened this issue May 16, 2019 · 4 comments · Fixed by #37341
Labels
Component: JavaScript Type: enhancement
Milestone
14.0.0

Comments

@asfimport
Copy link
Collaborator

Follow on work to ARROW-835

Reporter: Wes McKinney / @wesm

Note: This issue was originally created as ARROW-5356. Please see the migration documentation for further details.
@asfimport
Copy link
Collaborator Author

Wes McKinney / @wesm:
I moved this to the next release, but if it patch gets into 0.14.0 that's fine too

@asfimport
Copy link
Collaborator Author

Denis Gursky:
Is it supposed to work now? The Duration type seems to be implemented.

https://github.com/apache/arrow/blob/master/js/src/fb/duration.ts

But tableFromIPC throws: 'Unrecognized type: "Duration" (18)'

 

@asfimport
Copy link
Collaborator Author

Lukas Masuch:
I'm also running into the same problem ('Unrecognized type: "Duration" (18)'). Looking into the code, it seems that the Duration type is only partially implemented in javascript/typescript. For example, it fails to decode the duration type because it is missing in this switch case:  https://github.com/apache/arrow/blob/apache-arrow-9.0.0.dev/js/src/ipc/metadata/message.ts#L440

Not sure if this is intentional, a bug, or just not implemented yet?

@asfimport
Copy link
Collaborator Author

Todd Farmer / @toddfarmer:
This issue was last updated over 90 days ago, which may be an indication it is no longer being actively worked. To better reflect the current state, the issue is being unassigned per project policy. Please feel free to re-take assignment of the issue if it is being actively worked, or if you plan to start that work soon.

@kravets-levko kravets-levko mentioned this issue Mar 25, 2023
Merged
10 tasks
@jpkli jpkli mentioned this issue May 4, 2023
Closed
@FrNecas FrNecas mentioned this issue Aug 23, 2023
Merged
domoritz pushed a commit that referenced this issue Sep 28, 2023
@FrNecas @trxcllnt
GH-21815: [JS] Add support for Duration type (#37341)
befcc90 
### Rationale for this change

The `Duration` type is currently not supported and trying to deserialize a Table containing the type (e.g. using `tableFromIPC`) fails with `Unrecognized type` error. This PR aims to fix that.

### What changes are included in this PR?

- definition of the `Duration` data type
- updates to the visitor classes so that things like parsing work correctly
- test coverage for the type
- documentation update

### Are these changes tested?

Yes, I extended the data generator with the new type so that the type is tested by the existing tests.

### Are there any user-facing changes?

Yes, I've updated the documentation status page. I also noticed that it was outdated for JavaScript, i.e. there is already support for `Decimal` type so I updated this as well.

Closes: #21815
Closes: #35439
* Closes: #21815

Lead-authored-by: František Necas <frantisek.necas@protonmail.com>
Co-authored-by: ptaylor <paul.e.taylor@me.com>
Signed-off-by: Dominik Moritz <domoritz@gmail.com>
@domoritz domoritz added this to the 14.0.0 milestone Sep 28, 2023
JerAguilon pushed a commit to JerAguilon/arrow that referenced this issue Oct 23, 2023
@FrNecas @trxcllnt
apacheGH-21815: [JS] Add support for Duration type (apache#37341)
dcb6edb 
### Rationale for this change

The `Duration` type is currently not supported and trying to deserialize a Table containing the type (e.g. using `tableFromIPC`) fails with `Unrecognized type` error. This PR aims to fix that.

### What changes are included in this PR?

- definition of the `Duration` data type
- updates to the visitor classes so that things like parsing work correctly
- test coverage for the type
- documentation update

### Are these changes tested?

Yes, I extended the data generator with the new type so that the type is tested by the existing tests.

### Are there any user-facing changes?

Yes, I've updated the documentation status page. I also noticed that it was outdated for JavaScript, i.e. there is already support for `Decimal` type so I updated this as well.

Closes: apache#21815
Closes: apache#35439
* Closes: apache#21815

Lead-authored-by: František Necas <frantisek.necas@protonmail.com>
Co-authored-by: ptaylor <paul.e.taylor@me.com>
Signed-off-by: Dominik Moritz <domoritz@gmail.com>
JerAguilon pushed a commit to JerAguilon/arrow that referenced this issue Oct 23, 2023
@FrNecas @trxcllnt
apacheGH-21815: [JS] Add support for Duration type (apache#37341)
7e59061 
### Rationale for this change

The `Duration` type is currently not supported and trying to deserialize a Table containing the type (e.g. using `tableFromIPC`) fails with `Unrecognized type` error. This PR aims to fix that.

### What changes are included in this PR?

- definition of the `Duration` data type
- updates to the visitor classes so that things like parsing work correctly
- test coverage for the type
- documentation update

### Are these changes tested?

Yes, I extended the data generator with the new type so that the type is tested by the existing tests.

### Are there any user-facing changes?

Yes, I've updated the documentation status page. I also noticed that it was outdated for JavaScript, i.e. there is already support for `Decimal` type so I updated this as well.

Closes: apache#21815
Closes: apache#35439
* Closes: apache#21815

Lead-authored-by: František Necas <frantisek.necas@protonmail.com>
Co-authored-by: ptaylor <paul.e.taylor@me.com>
Signed-off-by: Dominik Moritz <domoritz@gmail.com>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
@FrNecas @trxcllnt @loicalleyne
apacheGH-21815: [JS] Add support for Duration type (apache#37341)
2046f23 
### Rationale for this change

The `Duration` type is currently not supported and trying to deserialize a Table containing the type (e.g. using `tableFromIPC`) fails with `Unrecognized type` error. This PR aims to fix that.

### What changes are included in this PR?

- definition of the `Duration` data type
- updates to the visitor classes so that things like parsing work correctly
- test coverage for the type
- documentation update

### Are these changes tested?

Yes, I extended the data generator with the new type so that the type is tested by the existing tests.

### Are there any user-facing changes?

Yes, I've updated the documentation status page. I also noticed that it was outdated for JavaScript, i.e. there is already support for `Decimal` type so I updated this as well.

Closes: apache#21815
Closes: apache#35439
* Closes: apache#21815

Lead-authored-by: František Necas <frantisek.necas@protonmail.com>
Co-authored-by: ptaylor <paul.e.taylor@me.com>
Signed-off-by: Dominik Moritz <domoritz@gmail.com>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
@FrNecas @trxcllnt @dgreiss
apacheGH-21815: [JS] Add support for Duration type (apache#37341)
d89e5f6 
### Rationale for this change

The `Duration` type is currently not supported and trying to deserialize a Table containing the type (e.g. using `tableFromIPC`) fails with `Unrecognized type` error. This PR aims to fix that.

### What changes are included in this PR?

- definition of the `Duration` data type
- updates to the visitor classes so that things like parsing work correctly
- test coverage for the type
- documentation update

### Are these changes tested?

Yes, I extended the data generator with the new type so that the type is tested by the existing tests.

### Are there any user-facing changes?

Yes, I've updated the documentation status page. I also noticed that it was outdated for JavaScript, i.e. there is already support for `Decimal` type so I updated this as well.

Closes: apache#21815
Closes: apache#35439
* Closes: apache#21815

Lead-authored-by: František Necas <frantisek.necas@protonmail.com>
Co-authored-by: ptaylor <paul.e.taylor@me.com>
Signed-off-by: Dominik Moritz <domoritz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Component: JavaScript Type: enhancement
Projects
None yet
Milestone
14.0.0
Development

Successfully merging a pull request may close this issue.

GH-21815: [JS] Add support for Duration type FrNecas/arrow
2 participants
@domoritz @asfimport