Skip to content
This repository was archived by the owner on Jun 14, 2024. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
612885a
add lineage to index records
Aug 5, 2020
1298e24
Merge branch 'master' into pouriap/lineage
Aug 5, 2020
495decf
Merge branch 'master' into pouriap/lineage
Aug 5, 2020
8f7c586
port benchmark code
Aug 5, 2020
4fdb8cc
misc cleanup in benchmark runner code
Aug 5, 2020
211a328
remove truncate from index sample record show
Aug 6, 2020
f8ed0d0
add misc to benchmark runner
Aug 6, 2020
edbc612
add more details to index stats report
Aug 6, 2020
f0e5383
Merge branch 'pouriap/benchmarkCode' into pouriap/lineage
Aug 6, 2020
74f1ed7
Revert "Merge branch 'pouriap/benchmarkCode' into pouriap/lineage"
Aug 6, 2020
a61e6a7
Merge branch 'master' into pouriap/lineage
Aug 6, 2020
5d8c118
add record lineage with tests - part1
Aug 7, 2020
6451b9a
Merge branch 'master' into pouriap/lineage
Aug 10, 2020
3cdb22f
remove lineage column from relation schema during transformation
Aug 10, 2020
11a0d1c
Modify rules to exclude lineage columns and add lineage tests
Aug 10, 2020
6522e0a
remove redundant import
Aug 11, 2020
b55c884
nit fix in test code
Aug 11, 2020
88a483c
Merge branch 'master' into pouriap/lineage
Aug 11, 2020
a5ec6d9
Change in CreateIndex tests
Aug 11, 2020
a9ed337
add lineage to index records with tests
Aug 18, 2020
c76a35f
Merge branch 'master' into pouriap/lineage
Aug 20, 2020
2083571
add lineage to index records
Aug 20, 2020
c8b80a3
add lineage to index records
Aug 20, 2020
e77b0ff
Merge branch 'master' into pouriap/lineage
Aug 20, 2020
2611362
Clean up lineage tests
Aug 22, 2020
7bd60de
clean up lineage tests
Aug 23, 2020
4b6b657
format fix in HyperspaceSuite
Aug 23, 2020
6b53112
Clean up lineage tests
Aug 23, 2020
7cdd529
nit
imback82 Aug 23, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@ package com.microsoft.hyperspace.actions

import java.io.FileNotFoundException

import org.apache.commons.io.FilenameUtils
import org.apache.hadoop.conf.Configuration
import org.apache.hadoop.fs.Path
import org.apache.spark.sql.{DataFrame, SparkSession}
import org.apache.spark.sql.execution.datasources.{HadoopFsRelation, LogicalRelation, PartitioningAwareFileIndex}
import org.apache.spark.sql.expressions.UserDefinedFunction
import org.apache.spark.sql.functions.{input_file_name, udf}
import org.apache.spark.sql.sources.DataSourceRegister

import com.microsoft.hyperspace.HyperspaceException
Expand Down Expand Up @@ -57,11 +60,8 @@ private[actions] abstract class CreateActionBase(dataManager: IndexDataManager)
val signatureProvider = LogicalPlanSignatureProvider.create()

// Resolve the passed column names with existing column names from the dataframe.
val (resolvedIndexedColumns, resolvedIncludedColumns) = resolveConfig(df, indexConfig)
val schema = {
val allColumns = resolvedIndexedColumns ++ resolvedIncludedColumns
df.select(allColumns.head, allColumns.tail: _*).schema
}
val (indexDataFrame, resolvedIndexedColumns, resolvedIncludedColumns) =
prepareIndexDataFrame(spark, df, indexConfig)

signatureProvider.signature(df.queryExecution.optimizedPlan) match {
case Some(s) =>
Expand All @@ -82,7 +82,7 @@ private[actions] abstract class CreateActionBase(dataManager: IndexDataManager)
CoveringIndex.Properties(
CoveringIndex.Properties
.Columns(resolvedIndexedColumns, resolvedIncludedColumns),
IndexLogEntry.schemaString(schema),
IndexLogEntry.schemaString(indexDataFrame.schema),
numBuckets)),
Content(
absolutePath.toString,
Expand Down Expand Up @@ -133,9 +133,7 @@ private[actions] abstract class CreateActionBase(dataManager: IndexDataManager)
IndexConstants.INDEX_NUM_BUCKETS_DEFAULT.toString)
.toInt

val (resolvedIndexedColumns, resolvedIncludedColumns) = resolveConfig(df, indexConfig)
val selectedColumns = resolvedIndexedColumns ++ resolvedIncludedColumns
val indexDataFrame = df.select(selectedColumns.head, selectedColumns.tail: _*)
val (indexDataFrame, resolvedIndexedColumns, _) = prepareIndexDataFrame(spark, df, indexConfig)

// run job
val repartitionedIndexDataFrame =
Expand Down Expand Up @@ -187,4 +185,47 @@ private[actions] abstract class CreateActionBase(dataManager: IndexDataManager)
s"from available source columns '${dfColumnNames.mkString(",")}'")
}
}

private def prepareIndexDataFrame(
spark: SparkSession,
df: DataFrame,
indexConfig: IndexConfig): (DataFrame, Seq[String], Seq[String]) = {
val (resolvedIndexedColumns, resolvedIncludedColumns) = resolveConfig(df, indexConfig)
val columnsFromIndexConfig = resolvedIndexedColumns ++ resolvedIncludedColumns
val addLineage = spark.sessionState.conf
.getConfString(
IndexConstants.INDEX_LINEAGE_ENABLED,
IndexConstants.INDEX_LINEAGE_ENABLED_DEFAULT)
.toBoolean

val indexDF = if (addLineage) {
// Lineage is captured using two sets of columns:
// 1. DATA_FILE_NAME_COLUMN column contains source data filename for each index record.
// 2. If source data is partitioned, all partitioning key(s) are added to index schema
// as columns if they are not already part of the schema.
val missingPartitionColumns = getPartitionColumns(df).filter(
ResolverUtils.resolve(spark, _, columnsFromIndexConfig).isEmpty)
val allIndexColumns = columnsFromIndexConfig ++ missingPartitionColumns
df.select(allIndexColumns.head, allIndexColumns.tail: _*)
.withColumn(IndexConstants.DATA_FILE_NAME_COLUMN, getFileName(input_file_name()))
} else {
df.select(columnsFromIndexConfig.head, columnsFromIndexConfig.tail: _*)
}

(indexDF, resolvedIndexedColumns, resolvedIncludedColumns)
}

private def getPartitionColumns(df: DataFrame): Seq[String] = {
// Extract partition keys, if original data is partitioned.
val partitionSchemas = df.queryExecution.optimizedPlan.collect {
case LogicalRelation(HadoopFsRelation(_, pSchema, _, _, _, _), _, _, _) => pSchema
}

Comment thread
imback82 marked this conversation as resolved.
// Currently we only support creating an index on a single LogicalRelation.
assert(partitionSchemas.length == 1)
partitionSchemas.head.map(_.name)
}

private val getFileName: UserDefinedFunction = udf(
(fullFilePath: String) => FilenameUtils.getName(fullFilePath))
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,8 @@ object IndexConstants {
val PLAIN_TEXT = "plaintext"
val HTML = "html"
}

private[hyperspace] val DATA_FILE_NAME_COLUMN = "_data_file_name"
val INDEX_LINEAGE_ENABLED = "spark.hyperspace.index.lineage.enabled"
val INDEX_LINEAGE_ENABLED_DEFAULT = "false"

@pirz pirz Aug 18, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to reviewers: For now, by default, I have disabled lineage for indexes - I already made sure that all test cases pass irrespective of this default value (i.e. all tests pass with this default value be true or false).

}
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ object FilterIndexRule
val newRelation = HadoopFsRelation(
newLocation,
new StructType(),
index.schema,
StructType(index.schema.filter(fsRelation.schema.contains)),
None, // Do not set BucketSpec to avoid limiting Spark's degree of parallelism
new ParquetFileFormat,
Map())(spark)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,26 +136,26 @@ object JoinIndexRule
* @return replacement plan
*/
private def getReplacementPlan(index: IndexLogEntry, plan: LogicalPlan): LogicalPlan = {
val bucketSpec = BucketSpec(
numBuckets = index.numBuckets,
bucketColumnNames = index.indexedColumns,
sortColumnNames = index.indexedColumns)

val location = new InMemoryFileIndex(spark, Seq(new Path(index.content.root)), Map(), None)
val relation = HadoopFsRelation(
location,
new StructType(),
index.schema,
Some(bucketSpec),
new ParquetFileFormat,
Map())(spark)

// Here we can't replace the plan completely with the index. This will create problems.
// For e.g. if Project(A,B) -> Filter(C = 10) -> Scan (A,B,C,D,E)
// if we replace this plan with index Scan (A,B,C), we lose the Filter(C=10) and it will
// lead to wrong results. So we only replace the base relation.
plan transform {
case baseRelation @ LogicalRelation(_: HadoopFsRelation, baseOutput, _, _) =>
val bucketSpec = BucketSpec(
numBuckets = index.numBuckets,
bucketColumnNames = index.indexedColumns,
sortColumnNames = index.indexedColumns)

val location = new InMemoryFileIndex(spark, Seq(new Path(index.content.root)), Map(), None)
val relation = HadoopFsRelation(
location,
new StructType(),
StructType(index.schema.filter(baseRelation.schema.contains(_))),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about defining a new function for this like index.originalSchema?

And I think index.schema.filter(field => field.name != IndexConstants.DATA_FILE_NAME_COLUMN) would be better?

@pirz pirz Aug 18, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, we only have one extra column and your suggested code works fine. However, I think it is better if we rely on the source data schema to pick schema for an index usage. This way if we add more extra column(s) later; this code works as expected with no change required.

@sezruby Please let me know if above is clear and makes sense to you.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed explanation, but it's a bit unclear to me. The current version also just uses index.schema, so it means the current version cannot handle the case you described?
I could be wrong, but this schema of HadoopFsRelation seems for reading index data and updatedOutput will filter unused columns out..?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As current version uses index.schema it has a similar issue, however I updated my comment as I realized this behavior is actually desired. The reason for that is if the source data at the query time uses a subset of the original source data then we should not use index as it could have extra rows that were part of original data and now excluded from the source data at query time. Imagine the index was created on source data with two partitions: .../data/A=a1 and .../data/A=a2. An index created on .../data path has records from both partitions a1 and a2. However, if data path at query time is set to .../data/A=a1 then it means records from the a2 partition should be excluded from the final results and if we use the index during query we may get those records back depending on the query and index config (i.e. potentially wrong query results). Moreover, our current signature computation/comparison does not mark such an index as a candidate for the query - and this is a correct behavior.
Regarding your point about HadoopFsRelation, if I understood what you mentioned correctly: Yes that schema is used for reading data; however any column in the schema is expected to be already resolved (i.e. has to have an ExprId) by the time we switch to an index. Once we switch to an index, we still use the same ExprId that Spark had assigned to the columns when analyzing the plan with source data. Hence, if the index schema has some column which was not present during former analyze phase, the query fails after transformation as such a column has no ExprId.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I got the point. I might also need to investigate ExprId related things for hybrid scan. Thanks :)

Some(bucketSpec),
new ParquetFileFormat,
Map())(spark)

val updatedOutput =
baseOutput.filter(attr => relation.schema.fieldNames.contains(attr.name))
baseRelation.copy(relation = relation, output = updatedOutput)
Expand Down
29 changes: 23 additions & 6 deletions src/test/scala/com/microsoft/hyperspace/SampleData.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.microsoft.hyperspace

import org.apache.spark.sql.SparkSession

/**
* Sample data for testing.
*/
Expand All @@ -25,10 +27,25 @@ object SampleData {
("2017-09-03", "fd093f8a05604515957083e70cb3dceb", "facebook", 1, 3000),
("2017-09-03", "af3ed6a197a8447cba8bc8ea21fad208", "facebook", 1, 3000),
("2017-09-03", "975134eca06c4711a0406d0464cbe7d6", "facebook", 1, 4000),
("2017-09-03", "e90a6028e15b4f4593eef557daf5166d", "zanahoria", 2, 3000),
("2017-09-03", "576ed96b0d5340aa98a47de15c9f87ce", "facebook", 2, 3000),
("2017-09-03", "50d690516ca641438166049a6303650c", "ibraco", 2, 1000),
("2017-09-03", "380786e6495d4cd8a5dd4cc8d3d12917", "facebook", 2, 3000),
("2017-09-03", "ff60e4838b92421eafc3e6ee59a9e9f1", "mi perro", 2, 2000),
("2017-09-03", "187696fe0a6a40cc9516bc6e47c70bc1", "multicines", 4, 3000))
("2018-09-03", "e90a6028e15b4f4593eef557daf5166d", "ibraco", 2, 3000),
("2018-09-03", "576ed96b0d5340aa98a47de15c9f87ce", "facebook", 2, 3000),
("2018-09-03", "50d690516ca641438166049a6303650c", "ibraco", 2, 1000),
("2019-10-03", "380786e6495d4cd8a5dd4cc8d3d12917", "facebook", 2, 3000),
("2019-10-03", "ff60e4838b92421eafc3e6ee59a9e9f1", "mi perro", 2, 2000),
("2019-10-03", "187696fe0a6a40cc9516bc6e47c70bc1", "facebook", 4, 3000))

def save(
spark: SparkSession,
path: String,
columns: Seq[String],
partitionColumns: Option[Seq[String]] = None): Unit = {
import spark.implicits._
val df = testData.toDF(columns: _*)
partitionColumns match {
case Some(pcs) =>
df.write.partitionBy(pcs: _*).parquet(path)
case None =>
df.write.parquet(path)
}
}
}
Loading