[SEDONA-223] Add ST_Split

[douglasdennis]

Did you read the Contributor Guide?

• Yes

Is this PR related to a JIRA ticket?

• Yes, the URL of the associated JIRA ticket is https://issues.apache.org/jira/browse/SEDONA-223. The PR name follows the format [SEDONA-XXX] my subject.

What changes were proposed in this PR?

1. Add ST_Split function to sedona-common, sedona-sql, sedona-flink, and the dataframe/python API.
2. Created FunctionsTest.java to house tests for functions being added to sedona-common.

How was this patch tested?

1. Comprehensive unit tests in sedona-common
2. Integration unit tests in sedona-sql, sedona-flink, and python.

Did this PR include necessary documentation updates?

• Yes, I have updated the documentation in docs/api/sql/Function.md.

[douglasdennis]

Ugh. I noticed that my VS Code seems to have gone wild on deleting leading white spaces. I hope that is ok. If the edits seem too messy though then I can try to revert them.

[douglasdennis]

I wasn't sure of a good way to access a log in sedona-common so I defaulted to standard error out. Is a logging mechanism something that needs to be added? @jiayuasu mentioned Log.error in the Jira ticket but I couldn't find the Log class/object. I'm happy to change this prior to merge if someone has a better way.

[jiayuasu]

You can use log4j for that. See an example here: https://github.com/apache/incubator-sedona/blob/master/core/src/main/java/org/apache/sedona/core/joinJudgement/JudgementBase.java#L120

[Kimahriman]

I was looking into standardizing the logging dependencies a little bit as a follow on to my last PR, can submit one for that soon. Right now log4j is available when spark is included, but not in common or flink

[jiayuasu]

Maybe we don't need to include log4j in the common module. Instead, functions in the common module only add throw exception in their func signatures and sedona-flink / sedona-spark catch them and produce log.error using log4j?

[douglasdennis]

Maybe a new type: SedonaFunctionException. With a constructor like: SedonaFunctionException(String message, Object result). Where message gets written to the appropriate log and result is the result of the function? I feel like there may sometimes be situations where a log entry needs to be made but a non-null result is still viable.

[Kimahriman]

I made #743 if we want to just be able to log in the common module

[Kimahriman]

And if you're going to add logging it might be a good chance to start using slf4j instead of log4j directly

[jiayuasu]

@douglasdennis PR #743 has been merged. If you pull from the upstream, you can actually now use log.error to handle the issue. Adam might need to make some changes later on but the dependencies should be the same.

[douglasdennis]

@jiayuasu Added it in. Let me know if I was supposed to do it some other way. The output I see looks like this:
23/01/04 23:52:03 ERROR GeometrySplitter: Colinear sections detected between source and blade geometry. Returned null.

[Kimahriman]

3 things:
Can you try changing the common dependency from

        <dependency>
            <groupId>org.apache.logging.log4j</groupId>
            <artifactId>log4j-1.2-api</artifactId>
        </dependency>

to

            <dependency>
                <groupId>org.slf4j</groupId>
                <artifactId>slf4j-api</artifactId>
            </dependency>

and try using the slf4j logger API instead? I think doing that in the common module would be a good place to try to stop using the log4j 1.x API directly.

Create the Logger as a static variable at the top of the class, I think that's the recommended way.

And kinda just my opinion, but if it doesn't break/stop the execution, I feel like it should be.a warning message and not an error message

[douglasdennis]

@Kimahriman Double check what I just pushed, please :)

I followed this which seemed to recommend using this LoggerFactory class as well. I don't mind iterating on this till you're good with it.

[Kimahriman]

Yep that's exactly what I was picturing!

[jiayuasu]

Awesome. Will merge.