Skip to content

ARROW-18208 [JS] Fixes crash when inferring objects with string-valued keys nested in an array#14554

Closed
MannySchneck wants to merge 4 commits intoapache:masterfrom
MannySchneck:cache-hack
Closed

ARROW-18208 [JS] Fixes crash when inferring objects with string-valued keys nested in an array#14554
MannySchneck wants to merge 4 commits intoapache:masterfrom
MannySchneck:cache-hack

Conversation

@MannySchneck
Copy link
Copy Markdown

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 1, 2022

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW

Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@MannySchneck MannySchneck changed the title cache infertype dictionary creation for consistent typeids ARROW-18208 [JS] Fixes crash when inferring objects with string-valued keys nested in an array Nov 1, 2022
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 1, 2022

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 1, 2022

⚠️ Ticket has no components in JIRA, make sure you assign one.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 1, 2022

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@MannySchneck
Copy link
Copy Markdown
Author

MannySchneck commented Nov 1, 2022

Reverted the code change and re-ran the test I wrote; test confirms the issue.

manny|arrow/js> yarn test
yarn run v1.22.19
$ cross-env NODE_NO_WARNINGS=1 gulp test
[20:50:59] Using gulpfile ~/code/arrow/js/gulpfile.js
[20:50:59] Starting 'test'...
[20:50:59] Starting 'test:ts'...
[20:50:59] Starting 'test:src'...
[20:50:59] Starting 'test:apache-arrow'...
[20:50:59] Starting 'test:es5:cjs'...
Determining test suites to run...Determining test suites to run...Determining test suites to run...Determini
ts-jest[ts-jest-transformer] (WARN) Define `ts-jest` config under `globals` is deprecated. Please do
transform: {
    <transform_regex>: ['ts-jest', { /* ts-jest config goes here in Jest */ }],
},
ts-jest[ts-jest-transformer] (WARN) Define `ts-jest` config under `globals` is deprecated. Please do
transform: {
    <transform_regex>: ['ts-jest', { /* ts-jest config goes here in Jest */ }],
},
ts-jest[ts-jest-transformer] (WARN) Define `ts-jest` config under `globals` is deprecated. Please do
transform: {
    <transform_regex>: ['ts-jest', { /* ts-jest config goes here in Jest */ }],
},
ts-jest[ts-jest-transformer] (WARN) Define `ts-jest` config under `globals` is deprecated. Please do
transform: {
    <transform_regex>: ['ts-jest', { /* ts-jest config goes here in Jest */ }],
},

  ● tableFromJSON() › creates table from objects with string values nested in an array

    TypeError: Unable to infer Vector type from input values, explicit type declaration expected

      166 |     }
      167 |
    > 168 |     throw new TypeError('Unable to infer Vector type from input values, explicit type declaration expected');
          |           ^
      169 | }
      170 |
      171 | /**

      at inferType (targets/ts/factories.ts:168:11)
      at inferType (targets/ts/factories.ts:161:52)
      at vectorFromArray (targets/ts/factories.ts:86:61)
      at tableFromJSON (targets/ts/factories.ts:101:20)
      at Object.<anonymous> (test/unit/table/table-test.ts:83:16)


  ● tableFromJSON() › creates table from objects with string values nested in an array

    TypeError: Unable to infer Vector type from input values, explicit type declaration expected

      166 |     }
      167 |
    > 168 |     throw new TypeError('Unable to infer Vector type from input values, explicit type declaration expected');
          |           ^
      169 | }
      170 |
      171 | /**

      at inferType (targets/apache-arrow/src/factories.ts:168:11)
      at inferType (targets/apache-arrow/src/factories.ts:161:52)
      at vectorFromArray (targets/apache-arrow/src/factories.ts:86:61)
      at tableFromJSON (targets/apache-arrow/src/factories.ts:101:20)
      at Object.<anonymous> (test/unit/table/table-test.ts:83:29)


@MannySchneck
Copy link
Copy Markdown
Author

This "fix" breaks duckdb, which is my use case, and as such is not really a "fix".

Uncaught (in promise) Error: Cannot create Schema containing two different dictionaries with the same Id
    at collectDictionaries (recordbatch.ts:329:23)
    at get dictionaries [as dictionaries] (recordbatch.ts:99:60)
    at RecordBatchStreamWriter._writeDictionaries (writer.ts:285:44)
    at RecordBatchStreamWriter._writeRecordBatch (writer.ts:254:14)
    at RecordBatchStreamWriter.write (writer.ts:183:22)
    at writeAll (writer.ts:441:16)
    at RecordBatchStreamWriter.writeAll (writer.ts:312:16)
    at Module.tableToIPC (serialization.ts:63:10)

@domoritz
Copy link
Copy Markdown
Member

Thanks for the pull request. Can you remove the comments and implement the right fix that also works in duckdb? I'm closing this pull request for now since it's incomplete but would be happy to reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants