Support globbing patterns in dataframes for creation/maintenance/usage of indexes.

[apoorvedave1]

What is the context for this pull request?

• Tracking Issue: [FEATURE REQUEST]: Support mutable source data when it's loaded with globbing pattern #226

What changes were proposed in this pull request?

Support for globbing patterns in data sources when creating/maintaining/using indexes.

Does this PR introduce any user-facing change?

Users will now be able to create and maintain indexes on dataframes created with globbing patterns e.g.

  val path = "tmp/1/*"
  val path2 = "tmp/2/*"
  val df = spark.read
    .option("spark.hyperspace.source.globbingPattern", s"$path,$path2") // This is only required the first time index is being created.
    .parquet(path, path2)

  val hs = new Hyperspace(spark)
  hs.createIndex(df, ...)

Note: for multiple paths, as shown in the example above, please concatenate the paths with commas ,.

How was this patch tested?

unit tests

[sezruby]

Minor comments, but LGTM!

[imback82]

LGTM (except for pending comments), thanks @apoorvedave1!

Could you also do a follow up PR to update doc (config + behavior)?

[sezruby]

Can you add create & refresh index tests for multiple * patterns ? e.g. table/*/m=1/*, table/*/* ..

[apoorvedave1]

thanks @sezruby , i added a test for multiple levels of /*/*.

For specialized case e.g. /*/m=1/*, i think it would be an overkill, because I mean if
spark.read.parquet("glob/*/m=1/*") works then this will also work right, as long as we pass in the right value in .parquet(<path>)? WDYT?

[sezruby]

You mean - is it required additional changes? or not necessary as it will work?

And could you add some tests for the other glob patterns? like

* (match 0 or more character)
? (match single character)
[ab] (character class)
[^ab] (negated character class)
[a-b] (character range)
{a,b} (alternation)
\c (escape character)

I think we should check if the index with these patterns will be refreshed correctly.

[apoorvedave1]

You mean - is it required additional changes? or not necessary as it will work?

And could you add some tests for the other glob patterns? like

* (match 0 or more character)
? (match single character)
[ab] (character class)
[^ab] (negated character class)
[a-b] (character range)
{a,b} (alternation)
\c (escape character)

I think we should check if the index with these patterns will be refreshed correctly.

I think it's overkill to add test for every regex possible since we are not adding any regex/wildcard handling logic at all.
cc @imback82 , @pirz , @rapoth for opinions?

[apoorvedave1]

I mean if spark.read.parquet("any-wildcard-pattern") works and we make sure that we are passing the same argument always, then it will work always right?

[sezruby]

Yeah but we only have the tests using * - we can add a test using all of those patterns are used.
I just worried about handling special characters in json & refresh index action, and not wanted to test regex behavior.

[imback82]

@sezruby if you only want a test handling special characters, we can just add it to JsonUtilsTests? I think we can address as a follow up PR.

[apoorvedave1]

Thanks, I have added this #284 issue for tracking json conversion tests

[imback82]

Cool, I am merging this to master. Let's do a follow up PR for this discussion. Thanks!

[apoorvedave1]

LGTM (except for pending comments), thanks @apoorvedave1!

Could you also do a follow up PR to update doc (config + behavior)?

Thanks @imback82 , This issue for tracking documentation. I will start the pr soon
#283

[imback82]

@apoorvedave1 Can you do a follow-up PR to add a test for the table case? I think you can create a table like the following: CREATE TABLE t(k int) USING json OPTIONS ('spark.hyperspace.source.globbingPattern', 'globbing path')

[imback82]

Can we also add the following case? Basically, we can inject this option after the table is created:

CREATE TABLE t(k int) USING json
ALTER TABLE t SET SERDEPROPERTIES ('spark.hyperspace.source.globbingPattern' = 'globbing pattern')
REFRESH TABLE t

[imback82]

@apoorvedave1 Could you take a look at the follow ups asked?