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.

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

[BUG] Function for Lead/Lag on Struct Type with default values does not work as expected. #8281

Closed
wbo4958 opened this issue May 19, 2021 · 6 comments
Assignees
Labels
bug Something isn't working libcudf Affects libcudf (C++/CUDA) code.

Comments

@wbo4958
Copy link
Contributor

wbo4958 commented May 19, 2021

Describe the bug

Function for Lead/Lag on Struct Type with default values does not work as expected.

Steps/Code to reproduce bug

I didn't dive into libcudf to check how to reproduce it, what I can do is copying java unit tests here for reproducing. You needs to copy the below code into TableTest.java and run the testWindowingLeadOnStruct test. This issue can be reproduced in the newest commit in branch-21.06

 @Test
  void testWindowingLeadOnStruct() {
    try (Table unsorted = new Table.TestBuilder()
        .column(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) // GBY Key
        .column(1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3) // GBY Key
        .column(1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6) // OBY Key
        .column(new StructType(true,
                new BasicType(true, DType.INT32),
                new BasicType(true, DType.STRING)),
            new StructData(1, "s1"), new StructData(null, "s2"), new StructData(2, null), new StructData(3, "s3"),
            new StructData(11, "s11"), null, new StructData(13, "s13"), new StructData(14, "s14"),
            new StructData(111, "s111"), new StructData(null, "s112"), new StructData(2, "s222"), new StructData(3, "s333")) //STRUCT Agg COLUMN
        .build()) {

      try (Table sorted = unsorted.orderBy(OrderByArg.asc(0), OrderByArg.asc(1), OrderByArg.asc(2))) {
        WindowOptions.Builder windowBuilder = WindowOptions.builder().minPeriods(1);
        try (Scalar zero = Scalar.fromInt(0);
             Scalar one = Scalar.fromInt(1);
             WindowOptions options = windowBuilder.window(zero, one).build();
             ColumnVector structDefaultOutput = ColumnVector.fromStructs(
                 new StructType(true,
                     new BasicType(true, DType.INT32),
                     new BasicType(true, DType.STRING)),
                 new StructData(-1, "s1"), new StructData(null, "s2"), new StructData(-2, null), new StructData(-3, "s3"),
                 new StructData(-11, "s11"), null, new StructData(-13, "s13"), new StructData(-14, "s14"),
                 new StructData(-111, "s111"), new StructData(null, "s112"), new StructData(-222, "s222"), new StructData(-333, "s333"));

             Table structWindowAggResults = sorted.groupBy(0, 1).aggregateWindows(
                 Aggregation
                     .lead(1, structDefaultOutput)
                     .onColumn(3) //STRUCT Agg COLUMN
                     .overWindow(options));
             ColumnVector structExpectAggResult = ColumnVector.fromStructs(
                 new StructType(true,
                     new BasicType(true, DType.INT32),
                     new BasicType(true, DType.STRING)),
                 new StructData(null, "s2"), new StructData(2, null), new StructData(3, "s3"), new StructData(-3, "s3"),
                 null, new StructData(13, "s13"), new StructData(14, "s14"), new StructData(-14, "s14"),
                 new StructData(null, "s112"), new StructData(2, "s222"), new StructData(3, "s333"), new StructData(-333, "s333"))) {
          // TODO  this is not gonna work, since libcudf has some issue for lead on struct with default values
           assertColumnsAreEqual(structExpectAggResult, structWindowAggResults.getColumn(0));
        }
      }
    }
  }

Expected behavior
The unit test should pass.

@wbo4958 wbo4958 added bug Something isn't working Needs Triage Need team to review and classify labels May 19, 2021
@wbo4958
Copy link
Contributor Author

wbo4958 commented May 19, 2021

@mythrocks pls help to check it

@mythrocks
Copy link
Contributor

Thanks for reporting this, @wbo4958. Looks like I don't have a libcudf test for the Struct case with defaults. Sorry I missed it.

@kkraus14 kkraus14 added libcudf Affects libcudf (C++/CUDA) code. and removed Needs Triage Need team to review and classify labels May 24, 2021
@beckernick
Copy link
Member

@wbo4958 @mythrocks was this resolved by #8507 ?

@wbo4958
Copy link
Contributor Author

wbo4958 commented Jun 17, 2021

Let me check today. Thx

@beckernick
Copy link
Member

Thanks Bobby!

@wbo4958
Copy link
Contributor Author

wbo4958 commented Jun 18, 2021

It worked. thx. close this issue

@wbo4958 wbo4958 closed this as completed Jun 18, 2021
rapids-bot bot pushed a commit that referenced this issue Jun 18, 2021
This PR is for #8281, since the bug has been fixed by #8507. So just enable the test.

Authors:
  - Bobby Wang (https://github.com/wbo4958)

Approvers:
  - Liangcai Li (https://github.com/firestarman)

URL: #8548
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Development

No branches or pull requests

4 participants