Delta diff: client side implementation of an expanded delta lake diff #5232

talSofer · 2023-02-14T08:20:26Z

Linked Issue

Change Description

New Feature

Add a GUI for showing the diff between two delta lake tables. the delta lake table diff view shows the diff for a single table at each point and colors the operations according to their type.

Testing Details

Tested manually the compare, commits and uncommitted changes views.
see attached screenshots:

Additional info

At this point (until we close Expanded delta diff view #5001) this feature is only visible when its feature flag is set to true.
The current pr relies on mock API responses and does not make any call to the backend. this will be changed after closing Delta Diff MVP: Endpoint impl #5228.

johnnyaug

Nice and reacty.
Have some comments.

johnnyaug · 2023-02-14T08:27:40Z

webui/src/lib/api/index.js

@@ -477,6 +477,20 @@ class Repositories {
            throw new Error(await extractError(response));
        }
    }
+
+    async otfDiff(repoId, leftRef, rightRef, table_path = "", type) {


Suggested change

async otfDiff(repoId, leftRef, rightRef, table_path = "", type) {

async otfDiff(repoId, leftRef, rightRef, tablePath = "", type) {

Not a fan of pushing commented out code, but if you're going with it, the argument should be commented out - or used in some fake way - or given as _.

changed naming, as for commenting out the code - i'm going to change in literally a few days so I think it is ok to keep it this way for now,

johnnyaug · 2023-02-14T08:28:53Z

webui/src/styles/globals.css

+
+.table-operation-expansion {
+  color: black;
+}


newline missing

johnnyaug · 2023-02-14T08:30:04Z

webui/src/lib/components/repository/TableDiff.jsx

+
+// The list of available operations is based on: https://docs.databricks.com/delta/history.html#operation-metrics-keys
+const deltaLakeOperationToDiffType = new Map([
+    ["WRITE" , DiffType.Added],


Looks like server side logic?

Why a Map and not a simple JS object?

I think that we've agreed that the type of diff will be returned from lakeFS within the response.
Created a PR to reflect it.

@johnnyaug good call, thanks! changed the client to use the operation type returned from the response

johnnyaug · 2023-02-14T08:34:40Z

webui/src/lib/components/repository/TableDiff.jsx

+    const diffType = deltaLakeOperationToDiffType.get(operation);
+    return diffType !== undefined ? diffType : DiffType.Changed;


Suggested change

const diffType = deltaLakeOperationToDiffType.get(operation);

return diffType !== undefined ? diffType : DiffType.Changed;

return deltaLakeOperationToDiffType.get(operation) || DiffType.Changed;

(and then this function is not really needed)

johnnyaug · 2023-02-14T08:37:14Z

webui/src/lib/components/repository/TableDiff.jsx

+]);
+
+export const TableDiff = ({repo, leftRef, rightRef, tablePath}) => {
+    let { error, loading, response } = useAPI(() => repositories.otfDiff(repo.id, leftRef, rightRef, tablePath, OtfType.Delta), [])


At this point this element becomes a DeltaDiff rather than a TableDiff.

true, renamed accordingaly

johnnyaug · 2023-02-14T08:38:17Z

webui/src/lib/components/repository/TableDiff.jsx

+        <td className="pl-lg-10 col-10 table-operation-details">
+            <strong>Timestamp:</strong> {operationTimestamp}
+            <br/>
+            <strong>Commit Info:</strong>


Where is the commit info here? Is it a lakeFS commit? (I hope so)

Commit info is the terminology used in the delta lake transaction log. see the definition in https://www.databricks.com/blog/2019/08/21/diving-into-delta-lake-unpacking-the-transaction-log.html

Commit info - Contains information around the commit, which operation was made, from where and at what time.

johnnyaug · 2023-02-14T08:41:05Z

webui/src/lib/components/repository/TableDiff.jsx

+    const JSONContentStr = JSON.stringify(content, null, 2);
+    const JSONContent = JSON.parse(JSONContentStr);


What is the purpose of these 2 lines? It looks like a no-op.

You are right, this is not needed, because I'm parsing here the following object included in the otfDiff response:

operation_content: type: object description: free form content describing the returned operation diff

and the object is a map of key-value pairs, so I don't need to convert it into a json first.
@Jonathan-Rosenberg should we clarify it in API doc? in it not relly a free form but a collection of key-value pairs.

johnnyaug · 2023-02-14T08:53:01Z

webui/src/lib/components/repository/TableDiff.jsx

+        parsedContent += `${key}: `
+        let parsedVal = "";
+        try {
+            parsedVal = JSON.parse(val);


This will look like [object Object] if val is parsed to a Javascript object.

It doesn't, it looks like this -
"[\"(spark_catalog.delta.lakefs://meetup-repo/experiment-2-branch/raw/.rating < 4.0D) aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"]"

parsed into

predicate: (spark_catalog.delta.lakefs://meetup-repo/experiment-2-branch/raw/.rating < 4.0D) aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

did you mean something else?

This code takes the result of JSON.parse and concatenates it with a string, consequently calling toString on the result, which returns [object Object] for a POJO.

johnnyaug · 2023-02-14T08:54:02Z

webui/src/lib/components/repository/TableDiff.jsx

+function parseOperationContent(content) {
+    let parsedContent = "";
+    const JSONContentStr = JSON.stringify(content, null, 2);
+    const JSONContent = JSON.parse(JSONContentStr);
+    for (let key in JSONContent) {
+        const val = JSONContent[key];
+        parsedContent += `${key}: `
+        let parsedVal = "";
+        try {
+            parsedVal = JSON.parse(val);
+        } catch (err) {
+            parsedVal = val;
+        }
+        parsedContent += parsedVal + "\n";
+    }
+    return parsedContent;


Separation of concerns - convert the object to what you want, handle the printing/formatting elsewhere.

Suggested change

function parseOperationContent(content) {

let parsedContent = "";

const JSONContentStr = JSON.stringify(content, null, 2);

const JSONContent = JSON.parse(JSONContentStr);

for (let key in JSONContent) {

const val = JSONContent[key];

parsedContent += `${key}: `

let parsedVal = "";

try {

parsedVal = JSON.parse(val);

} catch (err) {

parsedVal = val;

}

parsedContent += parsedVal + "\n";

}

return parsedContent;

function safeJsonParse(val) {

try {

return JSON.parse(val);

}

catch (err) {

return val;

}

}

function safeJsonParseValues(obj) {

Object.keys(obj).forEach(k => {

obj[k] = safeJsonParse(obj[k]);

})

}

johnnyaug · 2023-02-14T09:36:15Z

webui/src/lib/components/repository/changes.jsx

@@ -195,7 +207,9 @@ export const ChangesTreeContainer = ({results, showExperimentalDeltaDiffButton =
                                </span>
                        </Card.Header>
                        <Card.Body>
-                            <Table borderless size="sm">
+                            {tableDiffState.isExpanded


Because we went with separate Table/Object diff screens (or am I reading the code wrong?).

Suggested change

{tableDiffState.isExpanded

{tableDiffState.isShown

no you are good, done thx

Jonathan-Rosenberg · 2023-02-14T13:16:01Z

webui/src/constants.ts

+    Added = "added",
+    Removed = "removed",


Suggested change

Added = "added",

Removed = "removed",

Created = "created",

Dropped = "dropped",

done, I changed the enum to represent table operation types to be consistent with the server

Jonathan-Rosenberg · 2023-02-14T13:29:01Z

webui/src/lib/api/index.js

+        // }
+        // return response.json();
+        // const mockRes = '{"results": []}'
+        const mockRes = '{"results": [{"version": "1", "timestamp": 1515491537026, "operation": "INSERT", "operation_content": {"mode": "Append","partitionBy": "[]"}}, {"version": "2", "timestamp": 1515491537346, "operation": "DELETE", "operation_content": {"mode": "Append","partitionBy": "[]"}}, {"version": "14", "timestamp": 1674393286223, "operation": "DELETE", "operation_content": {"predicate": "[\\"(spark_catalog.delta.lakefs://meetup-repo/experiment-2-branch/raw/.`rating` < 4.0D) aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\"]"}}]}'


you're missing DiffType:

Suggested change

const mockRes = '{"results": [{"version": "1", "timestamp": 1515491537026, "operation": "INSERT", "operation_content": {"mode": "Append","partitionBy": "[]"}}, {"version": "2", "timestamp": 1515491537346, "operation": "DELETE", "operation_content": {"mode": "Append","partitionBy": "[]"}}, {"version": "14", "timestamp": 1674393286223, "operation": "DELETE", "operation_content": {"predicate": "[\\"(spark_catalog.delta.lakefs://meetup-repo/experiment-2-branch/raw/.`rating` < 4.0D) aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\"]"}}]}'

const mockRes = '{"diff_type": "changed", "results": [{"id": "1", "timestamp": 1515491537026, "operation": "INSERT", "operation_content": {"mode": "Append","partitionBy": "[]"}}, {"version": "2", "timestamp": 1515491537346, "operation": "DELETE", "operation_content": {"mode": "Append","partitionBy": "[]"}}, {"version": "14", "timestamp": 1674393286223, "operation": "DELETE", "operation_content": {"predicate": "[\\"(spark_catalog.delta.lakefs://meetup-repo/experiment-2-branch/raw/.`rating` < 4.0D) aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\"]"}}]}'

talSofer added 6 commits February 12, 2023 13:35

mock otfDiff api endpoint

e0492cd

implement expanded table diff view

e7b624c

add long example to test pasring and table styling

65ad847

global table diff styling

29fa3fb

destructure diff response for readability

f551083

move table diff styling to global css

6b26d15

talSofer added the exclude-changelog PR description should not be included in next release changelog label Feb 14, 2023

talSofer requested review from johnnyaug and Jonathan-Rosenberg February 14, 2023 08:20

johnnyaug approved these changes Feb 14, 2023

View reviewed changes

Jonathan-Rosenberg reviewed Feb 14, 2023

View reviewed changes

talSofer added 5 commits February 15, 2023 16:21

extract table operation type from otf diff response

21965a0

fix cr comments

f0b9362

separate operation content parsing from prop values parsing

2d81619

rename operation content parsing methods

d394019

change operation content strigify logic

ea94224

talSofer merged commit 2eae3a8 into master Feb 16, 2023

talSofer deleted the feature/delta-diff-phase-2 branch February 16, 2023 10:37

This was referenced Feb 22, 2023

Add mastodon icon and profile link to docs footer #5307

Merged

[DX] Web UI: Add spinner to setup screen button #5313

Merged

This was referenced Mar 6, 2023

Update/clarify S3 gateway config notes #5390

Merged

[docs] Add a little more detail to RBAC policy logic #5395

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delta diff: client side implementation of an expanded delta lake diff #5232

Delta diff: client side implementation of an expanded delta lake diff #5232

talSofer commented Feb 14, 2023

johnnyaug left a comment

johnnyaug Feb 14, 2023

talSofer Feb 15, 2023

johnnyaug Feb 14, 2023

talSofer Feb 15, 2023

johnnyaug Feb 14, 2023

johnnyaug Feb 14, 2023

Jonathan-Rosenberg Feb 14, 2023 •

edited

Loading

talSofer Feb 15, 2023

johnnyaug Feb 14, 2023

talSofer Feb 15, 2023

johnnyaug Feb 14, 2023

talSofer Feb 15, 2023

johnnyaug Feb 14, 2023

talSofer Feb 15, 2023

johnnyaug Feb 14, 2023

talSofer Feb 15, 2023

johnnyaug Feb 14, 2023

talSofer Feb 15, 2023

johnnyaug Feb 16, 2023 •

edited

Loading

johnnyaug Feb 14, 2023

talSofer Feb 16, 2023

johnnyaug Feb 14, 2023

talSofer Feb 15, 2023

Jonathan-Rosenberg Feb 14, 2023

talSofer Feb 15, 2023

Jonathan-Rosenberg Feb 14, 2023 •

edited

Loading

talSofer Feb 15, 2023

	async otfDiff(repoId, leftRef, rightRef, table_path = "", type) {
	async otfDiff(repoId, leftRef, rightRef, tablePath = "", type) {

		const diffType = deltaLakeOperationToDiffType.get(operation);
		return diffType !== undefined ? diffType : DiffType.Changed;

	const diffType = deltaLakeOperationToDiffType.get(operation);
	return diffType !== undefined ? diffType : DiffType.Changed;
	return deltaLakeOperationToDiffType.get(operation) \|\| DiffType.Changed;

		const JSONContentStr = JSON.stringify(content, null, 2);
		const JSONContent = JSON.parse(JSONContentStr);

Delta diff: client side implementation of an expanded delta lake diff #5232

Delta diff: client side implementation of an expanded delta lake diff #5232

Conversation

talSofer commented Feb 14, 2023

Linked Issue

Change Description

New Feature

Testing Details

Additional info

johnnyaug left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jonathan-Rosenberg Feb 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnnyaug Feb 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jonathan-Rosenberg Feb 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jonathan-Rosenberg Feb 14, 2023 •

edited

Loading

johnnyaug Feb 16, 2023 •

edited

Loading

Jonathan-Rosenberg Feb 14, 2023 •

edited

Loading