HPCC-31860 Test suite for the Parquet plugin #18980
Conversation
ilhan2316
changed the title
ilhan2316
force-pushed
the
HPCC-31860-Parquet-Plugin-Test-Suite
branch
from
905c5b2 to
d148f19
Compare
ilhan2316
changed the title
There was a problem hiding this comment.
@ilhan2316 I took a quick first look and had some questions and comments.
There were a few repeated style issues that I only pointed out a single example of, but make sure to check over all the files for things like ending newlines, correct copyright headers, etc.
Also, don't forget to remove any changes not associated with this PR.
Back to you.
ilhan2316
changed the title
There was a problem hiding this comment.
@ilhan2316 I think it is looking good. Don't be put off by the number of comments. Feel free to ask any questions that may come up.
Once you address these changes please take a look through the rest of the files to see if my comments apply in any other places. Once you are done I will look through the rest of the files.
There was a problem hiding this comment.
@ilhan2316 I just took a look at your ECL files and have a few comments.
|
|
||
| outputDataset := ParquetIO.Read(recordLayout, filePath); | ||
|
|
||
| compareDatasets := IF(COUNT(importedDataset) = COUNT(outputDataset), |
| ############################################################################## */ | ||
|
|
||
| //class=parquet | ||
| //Cover's data type's supported by ECL and arrow |
|
@ilhan2316 This will need to be rebased to the latest version of 9.8.x to pull in the changes from this JIRA. It contains changes to data type sizes and could cause some of your key files to change. |
ilhan2316
force-pushed
the
HPCC-31860-Parquet-Plugin-Test-Suite
branch
from
f50a03e to
92a925a
Compare
There was a problem hiding this comment.
@ilhan2316 Don't be put off by the number of comments it is getting close. Feel free to leave questions on GitHub and I can add examples and clarification.
Some files used in your test cases are missing from the PR as well as files that are included but not referenced in your ECL. I have included the list below. Also, make sure you check back to the PR to see the results of the smoketests. Some of the test cases are failing.
Files I didn't see a reference to in the ECL:
diverse.parquet
edgecase1.parquet
intial.parquet
integertest.parquet
medium.parquet
small.parquet
time3.parquet
updated.parquet
Files missing that were referenced:
single.parquet
multi_1_of_3.parquet
multi_2_of_3.parquet
multi_3_of_3.parquet
Missing key files:
parquet_types.xml
parquet_schema.xml
There was a problem hiding this comment.
@ilhan2316 Before I look at the changes could you please remove the files that shouldn't be there. You can refer to my earlier comment for the list of files that should be removed.
|
@ilhan2316 it looks like there are many of my comments that are still unresolved. Make sure you are going through the "Files Changed" section and opening each file to view my comments. Also, it helps me if you respond to the comments with what you changed, if you agree or disagree, or what your thoughts are rather than just resolving the thread. |
There was a problem hiding this comment.
@ilhan2316 I know this is a lot of comments, but I think they are overall easier to fix than they have previously been. If you want me to provide some ECL for these tests let me know and I can help you. Don't hesitate to ask questions either!
There was a problem hiding this comment.
@ilhan2316 It is very close. I left a few minor comments on parquetTypes.ecl. There were just a few additional cases that could be combined. There were also some files that should not be included. I made a comment of the ones I noticed.
There was a problem hiding this comment.
@ilhan2316 It looks very close. There are just a few places that need to use relative paths that I have commented on. It also looks like the parquetString.ecl is still failing.
| END; | ||
|
|
||
| result := JOIN(stringData, parquetString, | ||
| LEFT.s1 = RIGHT.s1 AND LEFT.s2 = RIGHT.s2 AND LEFT.s3 = RIGHT.s3, |
There was a problem hiding this comment.
This test is still failing, and I think the join condition needs testing. Add a //skip tag so the test will pass.
# Updated with changes Signed-off-by: Ilhan Gelle <[email protected]>
ilhan2316
force-pushed
the
HPCC-31860-Parquet-Plugin-Test-Suite
branch
from
bf55a5f to
8938abc
Compare
|
@ghalliday This is ready to merge. Please take a look. |
|
Jirabot Action Result: |
Type of change:
Checklist:
Smoketest:
Testing: