You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wayang.apache.org by "kmoltke (via GitHub)" <gi...@apache.org> on 2023/05/05 12:45:57 UTC

[GitHub] [incubator-wayang] kmoltke opened a new pull request, #318: Support sql query on multiple csv table files

kmoltke opened a new pull request, #318:
URL: https://github.com/apache/incubator-wayang/pull/318

   A minor change to SQL API to allow to run queries on multiple csv files. It is still hard coded for now, but this commit is a tiny improvement.


-- 
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-unsubscribe@wayang.apache.org

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


[GitHub] [incubator-wayang] kmoltke closed pull request #318: Support sql query on multiple csv table files

Posted by "kmoltke (via GitHub)" <gi...@apache.org>.
kmoltke closed pull request #318: Support sql query on multiple csv table files
URL: https://github.com/apache/incubator-wayang/pull/318


-- 
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-unsubscribe@wayang.apache.org

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


[GitHub] [incubator-wayang] kmoltke commented on pull request #318: Support sql query on multiple csv table files

Posted by "kmoltke (via GitHub)" <gi...@apache.org>.
kmoltke commented on PR #318:
URL: https://github.com/apache/incubator-wayang/pull/318#issuecomment-1544057028

   Closed due to new PR. Please see https://github.com/apache/incubator-wayang/pull/322 instead.


-- 
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-unsubscribe@wayang.apache.org

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


[GitHub] [incubator-wayang] CalvinKirs commented on a diff in pull request #318: Support sql query on multiple csv table files

Posted by "CalvinKirs (via GitHub)" <gi...@apache.org>.
CalvinKirs commented on code in PR #318:
URL: https://github.com/apache/incubator-wayang/pull/318#discussion_r1188168899


##########
wayang-api/wayang-api-sql/src/main/java/org/apache/wayang/api/sql/calcite/rel/WayangTableScan.java:
##########
@@ -78,7 +78,7 @@ public static WayangTableScan create(RelOptCluster cluster, RelOptTable relOptTa
 
     @Override
     public String toString() {
-        return "Wayang TableScan ["+ getQualifiedName() + "]";
+        return "Wayang TableScan [" + getQualifiedName() + "]";
     }
 

Review Comment:
   @2pk03 Do we have a unified code style check?



-- 
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-unsubscribe@wayang.apache.org

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


[GitHub] [incubator-wayang] kmoltke commented on pull request #318: Support sql query on multiple csv table files

Posted by "kmoltke (via GitHub)" <gi...@apache.org>.
kmoltke commented on PR #318:
URL: https://github.com/apache/incubator-wayang/pull/318#issuecomment-1541503520

   Hi, we spent several hours trying to do that. Unfortunately we had to prioritize finishing the report (we also agreed with Jorge about this). I believe it is possible though. Our problem was that it wasn't possible to access the file path through the RelNode. We were able to actually see the filesource during debugging, but the field was protected and not accessible. There is an issue on this: https://github.com/apache/incubator-wayang/issues/312 . Also, I think it would be good to create a new CsvTableScan class that extends the TableScan, so that the TableScan class will remain more generic (e.i. if source path is not required for a table scan). If we manage to somehow find time in the pretty packed exam period, we will definitely give it another go! :) 


-- 
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-unsubscribe@wayang.apache.org

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


[GitHub] [incubator-wayang] kmoltke commented on pull request #318: Support sql query on multiple csv table files

Posted by "kmoltke (via GitHub)" <gi...@apache.org>.
kmoltke commented on PR #318:
URL: https://github.com/apache/incubator-wayang/pull/318#issuecomment-1541526129

   Here's the screenshot of the debug that gave us headache ;) 
   
   We tried different casting and unwrapping us through the RelNode but with no luck..
   
   ![image](https://github.com/apache/incubator-wayang/assets/38263457/f9842a20-07da-47c3-843c-d5e6399f7866)


-- 
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-unsubscribe@wayang.apache.org

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


[GitHub] [incubator-wayang] zkaoudi commented on pull request #318: Support sql query on multiple csv table files

Posted by "zkaoudi (via GitHub)" <gi...@apache.org>.
zkaoudi commented on PR #318:
URL: https://github.com/apache/incubator-wayang/pull/318#issuecomment-1541628588

   Maybe adding a get() method to access the path could work? Not sure if it makes sense in general though..


-- 
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-unsubscribe@wayang.apache.org

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


[GitHub] [incubator-wayang] 2pk03 commented on a diff in pull request #318: Support sql query on multiple csv table files

Posted by "2pk03 (via GitHub)" <gi...@apache.org>.
2pk03 commented on code in PR #318:
URL: https://github.com/apache/incubator-wayang/pull/318#discussion_r1188169927


##########
wayang-api/wayang-api-sql/src/main/java/org/apache/wayang/api/sql/calcite/rel/WayangTableScan.java:
##########
@@ -78,7 +78,7 @@ public static WayangTableScan create(RelOptCluster cluster, RelOptTable relOptTa
 
     @Override
     public String toString() {
-        return "Wayang TableScan ["+ getQualifiedName() + "]";
+        return "Wayang TableScan [" + getQualifiedName() + "]";
     }
 

Review Comment:
   not yet, we should ;) 



-- 
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-unsubscribe@wayang.apache.org

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


[GitHub] [incubator-wayang] kmoltke commented on pull request #318: Support sql query on multiple csv table files

Posted by "kmoltke (via GitHub)" <gi...@apache.org>.
kmoltke commented on PR #318:
URL: https://github.com/apache/incubator-wayang/pull/318#issuecomment-1543712591

   Hi Zoi, we rearranged priorities with @JorgeQuiane and start working on getting rid of this hardcoded line. Just an fyi if it has an affect on the release plans :)


-- 
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-unsubscribe@wayang.apache.org

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