morningman commented on code in PR #24911: URL: https://github.com/apache/doris/pull/24911#discussion_r1359895277
########## fe/be-java-extensions/max-compute-scanner/src/main/java/org/apache/doris/maxcompute/MaxComputeJniScanner.java: ########## @@ -139,13 +145,24 @@ public void open() throws IOException { return; } try { - TableTunnel.DownloadSession session = curTableScan.getSession(); + TableTunnel.DownloadSession session; + if (partitionSpec != null) { + PartitionSpec partSpec = new PartitionSpec(partitionSpec); + session = curTableScan.openDownLoadSession(partSpec); + } else { + session = curTableScan.openDownLoadSession(); + } long start = startOffset == -1L ? 0 : startOffset; long recordCount = session.getRecordCount(); totalRows = splitSize > 0 ? Math.min(splitSize, recordCount) : recordCount; arrowAllocator = new RootAllocator(Long.MAX_VALUE); - curReader = session.openArrowRecordReader(start, totalRows, readColumns, arrowAllocator); + Set<String> partitionColumns = session.getSchema().getPartitionColumns().stream() + .map(Column::getName) + .collect(Collectors.toSet()); + List<Column> maxComputeColumns = new ArrayList<>(readColumns); + maxComputeColumns.removeIf(e -> partitionColumns.contains(e.getName())); Review Comment: Why remove partition columns here? ########## fe/be-java-extensions/max-compute-scanner/src/main/java/org/apache/doris/maxcompute/MaxComputeJniScanner.java: ########## @@ -124,6 +129,7 @@ protected void initTableInfo(ColumnType[] requiredTypes, String[] requiredFields } // reorder columns List<Column> columnList = curTableScan.getSchema().getColumns(); + columnList.addAll(curTableScan.getSchema().getPartitionColumns()); Review Comment: Does `curTableScan.getSchema().getColumns()` contains partition columns? ########## regression-test/suites/external_table_p2/maxcompute/test_external_catalog_maxcompute.groovy: ########## @@ -0,0 +1,55 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("test_external_catalog_maxcompute", "p2,external,maxcompute,external_remote,external_remote_maxcompute") { + String enabled = context.config.otherConfigs.get("enableMaxComputeTest") + if (enabled != null && enabled.equalsIgnoreCase("true")) { + String ak = getS3AK() Review Comment: Where to get ak and sk? It should be aliyun's ak and sk? ########## regression-test/suites/external_table_p2/maxcompute/test_external_catalog_maxcompute.groovy: ########## @@ -0,0 +1,55 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("test_external_catalog_maxcompute", "p2,external,maxcompute,external_remote,external_remote_maxcompute") { + String enabled = context.config.otherConfigs.get("enableMaxComputeTest") + if (enabled != null && enabled.equalsIgnoreCase("true")) { + String ak = getS3AK() + String sk = getS3SK() + String mc_catalog_name = "test_external_mc_catalog" + + sql """drop catalog if exists ${mc_catalog_name};""" + sql """ + create catalog if not exists ${mc_catalog_name} properties ( + "type" = "max_compute", + "mc.region" = "cn-beijing", + "mc.default.project" = "jz_datalake", + "mc.access_key" = "${ak}", + "mc.secret_key" = "${sk}", + "mc.public_access" = "true" + ); + """ + + def q01 = { + qt_q1 """ select count(*) from web_site """ + qt_q2 """ select count(*) from store_sales """ + qt_q3 """ select count(*) from mc_parts """ // partition table test + } + // type test + def q02 = { + qt_q4 """ select count(*) from int_types """ // test bool,tinyint,int,bigint + } + // test partition table filter + def q03 = { + qt_q5 """ select mc_string, dt from mc_parts where dt = 'dts' """ Review Comment: We need to add more complex cases, including: 1. all column types. 2. partition tables 3. complex where predicate including and/or/in combination -- 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: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org