Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix translation of types like xs:long to LongType, not IntegerType #522

Merged
merged 1 commit into from
Feb 18, 2021

Conversation

srowen
Copy link
Collaborator

@srowen srowen commented Feb 18, 2021

No description provided.

@srowen srowen added the bug label Feb 18, 2021
@srowen srowen added this to the 0.12.0 milestone Feb 18, 2021
@srowen srowen self-assigned this Feb 18, 2021
restriction.getBaseTypeName
}
matchType match {
simpleType.getQName match {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@seddonm1 this change seems to 'fix' the translation, which otherwise was coming up with xs:int when the schema says xs:long. This was from your original code, and I wasn't sure if there was another reason it has to be done this way? it's already known to be a simple type and seems like you can just get the QName in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I can't remember. At the time this was more a brute force reverse engineering process as the XSD spec is big.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Got it. I wouldn't want to break your use case - there are some tests but far from comprehensive. I'll try to think about why this would be needed, but I don't know XSDs that well (and at least I know it doesn't work in a simple case here). Thanks!

@codecov-io
Copy link

codecov-io commented Feb 18, 2021

Codecov Report

Merging #522 (63383ea) into master (f8d200c) will increase coverage by 0.18%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #522      +/-   ##
==========================================
+ Coverage   86.63%   86.82%   +0.18%     
==========================================
  Files          18       18              
  Lines         928      926       -2     
  Branches       81       82       +1     
==========================================
  Hits          804      804              
+ Misses        124      122       -2     
Impacted Files Coverage Δ
...la/com/databricks/spark/xml/util/XSDToSchema.scala 65.33% <100.00%> (+1.69%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8d200c...63383ea. Read the comment docs.

@srowen srowen merged commit e0b2739 into databricks:master Feb 18, 2021
@srowen srowen deleted the Issue520 branch February 19, 2021 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants