You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "zeroshade (via GitHub)" <gi...@apache.org> on 2023/04/23 15:46:17 UTC

[GitHub] [arrow] zeroshade commented on a diff in pull request #35178: GH-34852: [C++][Go][FlightRPC] Add support for ordered data

zeroshade commented on code in PR #35178:
URL: https://github.com/apache/arrow/pull/35178#discussion_r1174603139


##########
go/arrow/internal/flight_integration/scenario.go:
##########
@@ -526,6 +528,168 @@ func (m *middlewareScenarioTester) GetFlightInfo(ctx context.Context, desc *flig
 	}, nil
 }
 
+type orderedScenarioTester struct {
+	flight.BaseFlightServer
+}
+
+func (m *orderedScenarioTester) RunClient(addr string, opts ...grpc.DialOption) error {
+	client, err := flight.NewClientWithMiddleware(addr, nil, nil, opts...)
+	if err != nil {
+		return err
+	}
+	defer client.Close()
+
+	ctx := context.Background()
+	info, err := client.GetFlightInfo(ctx, &flight.FlightDescriptor{Type: flight.DescriptorCMD, Cmd: []byte("ordered")})
+	if err != nil {
+		return err
+	}
+
+	if !info.GetOrdered() {
+		return fmt.Errorf("expected to server return FlightInfo.ordered = true")
+	}
+
+	recs := make([]arrow.Record, len(info.Endpoint))
+	for i, ep := range info.Endpoint {
+		if len(ep.Location) != 0 {
+			return fmt.Errorf("expected to receive empty locations to use the original service: %s",
+				ep.Location)
+		}
+
+		stream, err := client.DoGet(ctx, ep.Ticket)
+		if err != nil {
+			return err
+		}
+
+		rdr, err := flight.NewRecordReader(stream)
+		if err != nil {
+			return err
+		}
+		defer rdr.Release()
+
+		for rdr.Next() {
+			record := rdr.Record()
+			record.Retain()
+			defer record.Release()
+			recs[i] = record
+		}
+		if rdr.Err() != nil {
+			return rdr.Err()
+		}
+	}
+
+	// Build expected records
+	mem := memory.DefaultAllocator
+	schema := arrow.NewSchema(
+		[]arrow.Field{
+			{Name: "number", Type: arrow.PrimitiveTypes.Int32},
+		},
+		nil,
+	)
+	expected_table, _ := array.TableFromJSON(mem, schema, []string{
+		`[
+                   {"number": 1},
+                   {"number": 2},
+                   {"number": 3}
+                 ]`,
+		`[
+                   {"number": 10},
+                   {"number": 20},
+                   {"number": 30}
+                 ]`,
+		`[
+                   {"number": 100},
+                   {"number": 200},
+                   {"number": 300}
+                 ]`,
+	})
+	defer expected_table.Release()
+
+	table := array.NewTableFromRecords(schema, recs)
+	defer table.Release()
+	if !array.TableEqual(table, expected_table) {
+		return fmt.Errorf("read data isn't expected\n" +
+			"Expected:\n" +
+			"%s\n" +
+			"num-rows: %d\n" +
+			"num-cols: %d\n" +
+			"Actual:\n" +
+			"%s\n" +
+			"num-rows: %d\n" +
+			"num-cols: %d",
+			expected_table.Schema(),
+			expected_table.NumRows(),
+			expected_table.NumCols(),
+			table.Schema(),
+			table.NumRows(),
+			table.NumCols())

Review Comment:
   I agree, we should definitely add a `String()` method to `arrow.Table`. Do you think it's worth adding it to this PR? or should we just make an issue and have it as a separate change?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org