You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2022/03/05 13:10:30 UTC
[GitHub] [drill] pjfanning opened a new pull request #2483: DRILL-8149: large xlsx configs
pjfanning opened a new pull request #2483:
URL: https://github.com/apache/drill/pull/2483
## Description
https://issues.apache.org/jira/browse/DRILL-8149
## Documentation
(Please describe user-visible changes similar to what should appear in the Drill documentation.)
## Testing
(Please describe how this PR has been tested.)
--
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: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on a change in pull request #2483: DRILL-8149: large xlsx configs
Posted by GitBox <gi...@apache.org>.
luocooong commented on a change in pull request #2483:
URL: https://github.com/apache/drill/pull/2483#discussion_r820607069
##########
File path: contrib/format-excel/src/main/java/org/apache/drill/exec/store/excel/ExcelBatchReader.java
##########
@@ -230,6 +239,18 @@ private void openFile(FileScanFramework.FileSchemaNegotiator negotiator) {
try {
fsStream = negotiator.fileSystem().openPossiblyCompressedStream(split.getPath());
+ if (readerConfig.maxArraySize >= 0) {
Review comment:
Is it possible to add the unit tests for these three options? we can refer to the : `TestExcelFormat#testLastRowQuery()`.
##########
File path: contrib/format-excel/src/main/java/org/apache/drill/exec/store/excel/ExcelFormatConfig.java
##########
@@ -98,6 +109,34 @@ public String getSheetName() {
return sheetName;
}
+ /**
+ * See the <code>setByteArrayMaxOverride</code> section in the Apache POI
Review comment:
Is it possible to update the `drill/contrib/format-excel/README.md` file?
--
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: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2483: DRILL-8149: large xlsx configs
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2483:
URL: https://github.com/apache/drill/pull/2483#issuecomment-1077725853
>
No worries! It looks good to me other than adding some doc updates.
--
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: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] pjfanning commented on pull request #2483: DRILL-8149: large xlsx configs
Posted by GitBox <gi...@apache.org>.
pjfanning commented on pull request #2483:
URL: https://github.com/apache/drill/pull/2483#issuecomment-1077722983
@cgivre @luocooong thanks for the feedback. I've been busy on other stuff and am not prioritising completing this. Before it is ready to merge, it will need doc updates and test coverage.
--
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: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] pjfanning commented on pull request #2483: DRILL-8149: large xlsx configs
Posted by GitBox <gi...@apache.org>.
pjfanning commented on pull request #2483:
URL: https://github.com/apache/drill/pull/2483#issuecomment-1059762233
Still needs test coverage
--
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: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on a change in pull request #2483: DRILL-8149: large xlsx configs
Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2483:
URL: https://github.com/apache/drill/pull/2483#discussion_r834400430
##########
File path: contrib/format-excel/src/main/java/org/apache/drill/exec/store/excel/ExcelBatchReader.java
##########
@@ -183,6 +189,9 @@ public String getFieldName() {
lastColumn = plugin.getConfig().getLastColumn();
allTextMode = plugin.getConfig().getAllTextMode();
sheetName = plugin.getConfig().getSheetName();
+ maxArraySize = plugin.getConfig().getMaxArraySize();
Review comment:
@pjfanning Thanks for the update! Can you update the `README` file in this folder to explain what these options do? Do you have recommended values?
--
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: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org