parthchandra commented on code in PR #1359:
URL: https://github.com/apache/datafusion-comet/pull/1359#discussion_r1939889625


##########
native/core/src/execution/planner.rs:
##########
@@ -1155,12 +1154,9 @@ impl PhysicalPlanner {
                     ))
                 });
 
-                let object_store = object_store::local::LocalFileSystem::new();
-                // register the object store with the runtime environment
-                let url = Url::try_from("file://").unwrap();
-                self.session_ctx
-                    .runtime_env()
-                    .register_object_store(&url, Arc::new(object_store));
+                // By default, local FS object store registered
+                // if `hdfs` feature enabled then HDFS file object store 
registered
+                let object_store_url = 
register_object_store(Arc::clone(&self.session_ctx))?;

Review Comment:
   We don't need to wait for actual iceberg integration. CometScan will use 
COMPAT_ICEBERG if the configuration is set (That's how we are able to run the 
unit tests). 



##########
Makefile:
##########
@@ -95,7 +98,7 @@ release-linux: clean
        cd native && RUSTFLAGS="-Ctarget-cpu=native 
-Ctarget-feature=-prefer-256-bit" cargo build --release
        ./mvnw install -Prelease -DskipTests $(PROFILES)
 release:
-       cd native && RUSTFLAGS="-Ctarget-cpu=native" cargo build --release
+       cd native && RUSTFLAGS="$(RUSTFLAGS) -Ctarget-cpu=native" && 
RUSTFLAGS=$$RUSTFLAGS cargo build --release $(FEATURES_ARG)

Review Comment:
   Thanks. Learnt something new today :) 



##########
native/core/Cargo.toml:
##########
@@ -77,6 +77,7 @@ datafusion-comet-proto = { workspace = true }
 object_store = { workspace = true }
 url = { workspace = true }
 chrono = { workspace = true }
+datafusion-objectstore-hdfs = { git = 
"https://github.com/comphead/datafusion-objectstore-hdfs";, branch = "master", 
optional = true }

Review Comment:
   Is there an expected timeline for when we can move to an official release? 
Meantime, since we have pointed to a personal repo in the past, it is 
reasonable to do so for this as well (especially since this is already behind 
some configuration flags).



-- 
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-unsubscr...@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to