GH-35243: [C#] Implement MapType by CurtHagenlocher · Pull Request #37885 · apache/arrow

CurtHagenlocher · 2023-09-26T18:18:40Z

What changes are included in this PR?

This change includes the original work by @Platob from #35263, updated with the latest changes in the main branch. Additionally, it includes support in the C API for the Map type and fills a few gaps in functionality from the original change (for instance, ArrowArrayConcentrator support). Finally, it fixes a bug in the ArrowArrayConcentrator.Concatenate which prevented it from working correctly for structs. (This bug was discovered in the course of creating a test for concatenating maps.)

Are these changes tested?

Yes.

Are there any user-facing changes?

Maps are now supported in the C# implementation.

Closes #35243

Closes: [C#] Implement MapType #35243

Fix concatenation of Structs to work correctly.

davidhcoe

a few nits on comments but looks good

csharp/test/Apache.Arrow.Tests/ArrowArrayConcatenatorTests.cs

csharp/src/Apache.Arrow/C/CArrowSchemaImporter.cs

davidhcoe · 2023-10-04T21:31:11Z

@lidavidm - are you able to help at all?

lidavidm · 2023-10-04T22:21:26Z

Thanks for the ping - I'll look at this tomorrow!

lidavidm

LGTM, thanks!

I restarted the integration pipeline so let's make sure things pass there.

lidavidm · 2023-10-05T13:15:43Z

Hmm, there appears to be something wrong with the pipeline

Building wheels for collected packages: pythonnet
  Building wheel for pythonnet (setup.py): started
  Building wheel for pythonnet (setup.py): finished with status 'error'
  error: subprocess-exited-with-error
  
  × python setup.py bdist_wheel did not run successfully.
  │ exit code: 1
  ╰─> [59 lines of output]
      /tmp/pip-install-m_41mbf1/pythonnet_3828907f28a2453cac8f6ff2d84ed995/setup.py:468: SyntaxWarning: invalid escape sequence '\*'
        "MSBuild\**\Bin\MSBuild.exe",
      /tmp/pip-install-m_41mbf1/pythonnet_3828907f28a2453cac8f6ff2d84ed995/setup.py:533: SyntaxWarning: invalid escape sequence '\*'
        "MSBuild\**\Bin\MSBuild.exe",
      running bdist_wheel
      running build
      running build_ext
      /bin/sh: 1: mono: not found

lidavidm · 2023-10-05T13:22:22Z

Ah, weird, it tried to install 2.5.2 instead of 3.0.2.

@CurtHagenlocher can you rebase/merge one last time? It appears we recently had to pin Python to keep pythonnet working

CurtHagenlocher · 2023-10-05T16:13:10Z

Looks like another infrastructure problem

################# FAILURES #################
FAILED TEST: recursive_nested Rust producing,  Rust consuming
<class 'RuntimeError'>: Command failed: /arrow/rust/target/debug/flight-test-integration-client --host localhost --port=46587 --path /tmp/arrow-integration-o7e_thtg/generated_recursive_nested.json
With output:
--------------
Error: tonic::transport::Error(Transport, hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 99, kind: AddrNotAvailable, message: "Cannot assign requested address" })))

lidavidm · 2023-10-05T16:32:21Z

Well, the C# tests pass so I think let's merge this.

conbench-apache-arrow · 2023-10-06T17:54:37Z

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 4b795ec.

There was 1 benchmark result indicating a performance regression:

Commit Run on arm64-m6g-linux-compute at 2023-10-06 07:54:03Z
- dataset-select (Python) with dataset=nyctaxi_multi_parquet_s3_repartitioned

The full Conbench report has more details. It also includes information about 6 possible false positives for unstable benchmarks that are known to sometimes produce them.

### What changes are included in this PR? This change includes the original work by `@ Platob` from apache#35263, updated with the latest changes in the main branch. Additionally, it includes support in the C API for the Map type and fills a few gaps in functionality from the original change (for instance, ArrowArrayConcentrator support). Finally, it fixes a bug in the ArrowArrayConcentrator.Concatenate which prevented it from working correctly for structs. (This bug was discovered in the course of creating a test for concatenating maps.) ### Are these changes tested? Yes. ### Are there any user-facing changes? Maps are now supported in the C# implementation. Closes apache#35243 * Closes: apache#35243 Lead-authored-by: Platob <nfillot.pro@gmail.com> Co-authored-by: Curt Hagenlocher <curt@hagenlocher.org> Signed-off-by: David Li <li.davidm96@gmail.com>

Platob and others added 19 commits April 21, 2023 09:13

MapType : Init type and array

f4778f5

ListType : seal type

e159ca9

MapType : Add IPC tests

d11d75b

code : clean

3c5b5b9

code : clean

10ae223

MapArray : Add builder test

220bd97

MapArray : test netcore 3 1 constraint

96b3965

MapArray : inner struct not null array

a6b927a

MapArray : GetKeyValuePairs with constructor

093a248

MapArray : fix KeyValues slice

acc2bc7

MapArray : GetTuple

e403994

code : clean

62013c1

MapArray : rename GetTuple to GetTuples

571cb5e

MapArray : IPC get meta

c9b4694

rm #if NETCOREAPP3_1_OR_GREATER constraint

fb01fe5

Merge from main

d06c753

Support C API for maps

eb0e3d0

Finish implementing Map.

bc703d6

Fix concatenation of Structs to work correctly.

One fix plus one test

af69e08

CurtHagenlocher requested review from assignUser, kou, raulcd and westonpace as code owners September 26, 2023 18:18

github-actions bot added Component: C# Component: Documentation awaiting review Awaiting review labels Sep 26, 2023

CurtHagenlocher added 3 commits September 26, 2023 11:39

Add missing copyright header

396427c

Make "keysSorted" a little more optional.

87faaf6

Actually fix "KeysSorted" problem.

cef8911

davidhcoe reviewed Sep 27, 2023

View reviewed changes

csharp/test/Apache.Arrow.Tests/ArrowArrayConcatenatorTests.cs Outdated Show resolved Hide resolved

csharp/src/Apache.Arrow/C/CArrowSchemaImporter.cs Outdated Show resolved Hide resolved

westonpace mentioned this pull request Sep 27, 2023

GH-35243: [C#] Implement MapType #35263

Closed

davidhcoe mentioned this pull request Sep 27, 2023

feat(csharp): Add ADO.NET client; tests for C# to interop with the Snowflake Go driver apache/arrow-adbc#1031

Merged

CurtHagenlocher added 2 commits October 2, 2023 06:50

Address code review feedback

addd081

Merge branch 'Map' of https://github.com/CurtHagenlocher/arrow into Map

d2377d6

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 2, 2023

CurtHagenlocher added 3 commits October 2, 2023 06:51

Fixed comment

4f7e548

Merge from main

cfaff68

Restore changes lost to the merge

5316971

CurtHagenlocher requested a review from westonpace October 2, 2023 20:48

lidavidm approved these changes Oct 5, 2023

View reviewed changes

github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Oct 5, 2023

Merge branch 'main' of https://github.com/CurtHagenlocher/arrow into Map

9ae4a0c

lidavidm merged commit 4b795ec into apache:main Oct 5, 2023

lidavidm removed the awaiting merge Awaiting merge label Oct 5, 2023

CurtHagenlocher deleted the Map branch October 16, 2023 16:28

CurtHagenlocher mentioned this pull request Sep 5, 2023

[C#] Support new data types #34736

Closed

9 tasks

abandy added a commit to abandy/arrow that referenced this pull request Dec 5, 2023

apacheGH-37885: [Swift] allow reading of unaligned flatbuffer buffers

b9f9ffb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-35243: [C#] Implement MapType#37885

GH-35243: [C#] Implement MapType#37885
lidavidm merged 28 commits intoapache:mainfrom
CurtHagenlocher:Map

CurtHagenlocher commented Sep 26, 2023 •

edited by lidavidm

Loading

Uh oh!

davidhcoe left a comment

Uh oh!

Uh oh!

Uh oh!

davidhcoe commented Oct 4, 2023

Uh oh!

lidavidm commented Oct 4, 2023

Uh oh!

lidavidm left a comment

Uh oh!

lidavidm commented Oct 5, 2023

Uh oh!

lidavidm commented Oct 5, 2023 •

edited

Loading

Uh oh!

CurtHagenlocher commented Oct 5, 2023 •

edited

Loading

Uh oh!

lidavidm commented Oct 5, 2023

Uh oh!

conbench-apache-arrow bot commented Oct 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

CurtHagenlocher commented Sep 26, 2023 • edited by lidavidm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

davidhcoe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

davidhcoe commented Oct 4, 2023

Uh oh!

lidavidm commented Oct 4, 2023

Uh oh!

lidavidm left a comment

Choose a reason for hiding this comment

Uh oh!

lidavidm commented Oct 5, 2023

Uh oh!

lidavidm commented Oct 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CurtHagenlocher commented Oct 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lidavidm commented Oct 5, 2023

Uh oh!

conbench-apache-arrow bot commented Oct 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

CurtHagenlocher commented Sep 26, 2023 •

edited by lidavidm

Loading

lidavidm commented Oct 5, 2023 •

edited

Loading

CurtHagenlocher commented Oct 5, 2023 •

edited

Loading