Skip to content

Conversation

@CarolinePeng
Copy link

What changes were proposed in this pull request?

When using ordinals to access linked list, the time cost is O(n).

How was this patch tested?

Existing tests.

@HyukjinKwon
Copy link
Member

It's okay but mind taking another look and fix some more typos together while we're here? I'm pretty sure there are more.

StructType(attrs.map(a => StructField(a.name, a.dataType, a.nullable, a.metadata)))
}

// It's possible that `attrs` is a linked list, which can lead to bad O(n^2) loops when
Copy link
Member

@dongjoon-hyun dongjoon-hyun Dec 11, 2018

Choose a reason for hiding this comment

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

Could you tell us why you think this comment is error? This comments came from SPARK-15764 Replace N^2 loop in BindReferences to explain the problamatic situation.

When using ordinals to access linked list, the time cost is O(n).

Copy link
Author

Choose a reason for hiding this comment

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

The cost time to access the linked list using ordinal is O (n), but I do not combine BindReferences to analyze the cost time.

Copy link
Member

Choose a reason for hiding this comment

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

Ya. The original sentence has a different context at that time.

@SparkQA
Copy link

SparkQA commented Dec 12, 2018

Test build #4464 has finished for PR 23280 at commit fdb1f3b.

  • This patch fails Spark unit tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Dec 14, 2018

Test build #4470 has finished for PR 23280 at commit fdb1f3b.

  • This patch passes all tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Dec 14, 2018

The merge error is spurious; merging to master

@srowen srowen closed this in d25e443 Dec 14, 2018
holdenk pushed a commit to holdenk/spark that referenced this pull request Jan 5, 2019
## What changes were proposed in this pull request?

When using ordinals to access linked list, the time cost is O(n).

## How was this patch tested?

Existing tests.

Closes apache#23280 from CarolinePeng/update_Two.

Authored-by: CarolinPeng <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
## What changes were proposed in this pull request?

When using ordinals to access linked list, the time cost is O(n).

## How was this patch tested?

Existing tests.

Closes apache#23280 from CarolinePeng/update_Two.

Authored-by: CarolinPeng <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants