[Feat][Spark] Refactoring of the Maven project to allow multiple spark versions in the same moment

[SemyonSinchenko]

Proposed changes

Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

• I have read the CONTRIBUTING doc
• I have signed the CLA
• Lint and unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

[SemyonSinchenko]

The idea is to have the following structure:

| root
| pom.xml
| datasources-v1
|- pom.xml
|- src/main/scala/com/alibaba/...
| datasources-v2
|- pom.xml
|- src/main/scala/com/alibaba/...
| graphar
|- pom.xml
|- src/main/scala/com/alibaba/...

Inside graphar submodule only commons code is placed;
Inside datasources-v1/v2 datasources for spark <=3.2 and for spark >=3.3 (when they switched to v2 for orc/parquet readers) are placed.

The code is locally compiling successfully for both profiles. This is a DRAFT! So, @acezen please make a look on pom files and overall structure. I do not want go deeper without a discussion of the overall concept. Thanks!

[acezen]

Thanks for the work! I will review it ASAP.

[SemyonSinchenko]

Test files are here just by accident, looks like I accidentally deleted a git-submodule by copying. I will clean it, of course.

[acezen]

Test files are here just by accident, looks like I accidentally deleted a git-submodule by copying. I will clean it, of course.

sorry, Sem, I am currently on a short vacation, and for the next couple of days, I will likely be unable to review this PR. Once I return from my break, I will promptly review it.

[SemyonSinchenko]

Another problem that I found is that parquet-reader in 3.2 is incompatible with parquet-reader in 3.3+. The configuration ("spark.sql.legacy.parquet.nanosAsLong", "false") should be added to SparkSession. Because SparkSession is always created by user, I have no idea how to fix it.

[acezen]

It seems that the GeneralParams could be the common part of GraphAr-spark, does we need to duplicated it in datasource?

[SemyonSinchenko]

They are used inside data sources, and I duplicate it to avoid circular dependency (common depends of datasources because of Neo4j and if datasources will depend of common we will have a circular dependency)

[acezen]

ok

[acezen]

Another problem that I found is that parquet-reader in 3.2 is incompatible with parquet-reader in 3.3+. The configuration ("spark.sql.legacy.parquet.nanosAsLong", "false") should be added to SparkSession. Because SparkSession is always created by user, I have no idea how to fix it.

Maybe we update the spark config of SparkSession when it needs the config ("spark.sql.legacy.parquet.nanosAsLong", "false"):

def UDF(spark: SparkSession): 
     spark.conf.set("spark.sql.legacy.parquet.nanosAsLong", "false")
     # assume that the application below need the config to be set

[acezen]

The idea is to have the following structure:

| root
| pom.xml
| datasources-v1
|- pom.xml
|- src/main/scala/com/alibaba/...
| datasources-v2
|- pom.xml
|- src/main/scala/com/alibaba/...
| graphar
|- pom.xml
|- src/main/scala/com/alibaba/...

Inside graphar submodule only commons code is placed; Inside datasources-v1/v2 datasources for spark <=3.2 and for spark >=3.3 (when they switched to v2 for orc/parquet readers) are placed.

The code is locally compiling successfully for both profiles. This is a DRAFT! So, @acezen please make a look on pom files and overall structure. I do not want go deeper without a discussion of the overall concept. Thanks!

the pom and overall structure look good to me, I have a small question that how the datasource version switch work when user compile with different spark version?

[SemyonSinchenko]

The idea is to have the following structure:

| root
| pom.xml
| datasources-v1
|- pom.xml
|- src/main/scala/com/alibaba/...
| datasources-v2
|- pom.xml
|- src/main/scala/com/alibaba/...
| graphar
|- pom.xml
|- src/main/scala/com/alibaba/...

Inside graphar submodule only commons code is placed; Inside datasources-v1/v2 datasources for spark <=3.2 and for spark >=3.3 (when they switched to v2 for orc/parquet readers) are placed.

The code is locally compiling successfully for both profiles. This is a DRAFT! So, @acezen please make a look on pom files and overall structure. I do not want go deeper without a discussion of the overall concept. Thanks!

the pom and overall structure look good to me, I have a small question that how the datasource version switch work when user compile with different spark version?

Users should use maven profiles (spark32, spark33, etc.), defined in top level pom.xml. Spark version is specified inside maven profiles

[SemyonSinchenko]

@acezen By the latest commit I introduced changes that make all tests passed. But I still do not fully understand the logic inside spark/srcipts and how should I change it. The same about CI/CD. So, long story short, I need some help here :)

[acezen]

@acezen By the latest commit I introduced changes that make all tests passed. But I still do not fully understand the logic inside spark/srcipts and how should I change it. The same about CI/CD. So, long story short, I need some help here :)

The spark/scripts just contain some bash script that deploy neo4j/nebula environment and run application in https://github.com/alibaba/GraphAr/tree/main/spark/src/main/scala/com/alibaba/graphar/example, so it would not affect by spark version and no need to modify.

[acezen]

hi, Sem , can we make this refactoring by splitting the changes to some small PR that avoid hard to back tracing. Maybe we can start from separate the datasource part as a standalone package.

[SemyonSinchenko]

@acezen By the latest commit I introduced changes that make all tests passed. But I still do not fully understand the logic inside spark/srcipts and how should I change it. The same about CI/CD. So, long story short, I need some help here :)

The spark/scripts just contain some bash script that deploy neo4j/nebula environment and run application in https://github.com/alibaba/GraphAr/tree/main/spark/src/main/scala/com/alibaba/graphar/example, so it would not affect by spark version and no need to modify.

There is a mvn command inside, and it won't work after introducing maven profiles. Now, each mvn command should also contain -P datasources-32 or -P datasources-33. Which one should I add? Or should I add both of them? And the same question about unit tests. And the top level question is how to add all these things into CI. If you can draw on a white sheet some kind of top-level structure, it could be cool and I will implement it!

[SemyonSinchenko]

hi, Sem , can we make this refactoring by splitting the changes to some small PR that avoid hard to back tracing. Maybe we can start from separate the datasource part as a standalone package.

Ok, I will try. I will start from the only refactoring (datasources-32). And I will add datasources-33 in the next PR.

[acezen]

@acezen By the latest commit I introduced changes that make all tests passed. But I still do not fully understand the logic inside spark/srcipts and how should I change it. The same about CI/CD. So, long story short, I need some help here :)

The spark/scripts just contain some bash script that deploy neo4j/nebula environment and run application in https://github.com/alibaba/GraphAr/tree/main/spark/src/main/scala/com/alibaba/graphar/example, so it would not affect by spark version and no need to modify.

There is a mvn command inside, and it won't work after introducing maven profiles. Now, each mvn command should also contain -P datasources-32 or -P datasources-33. Which one should I add? Or should I add both of them? And the same question about unit tests. And the top level question is how to add all these things into CI. If you can draw on a white sheet some kind of top-level structure, it could be cool and I will implement it!

The top-level structure looks like:

[SemyonSinchenko]

I will close this PR and split the work to smaller PRs.

[SemyonSinchenko]

#369