Skip to content

Fixed incorrect result type of duration-duration division#229

Merged
Jak-MS merged 1 commit intoMicrosoftDocs:mainfrom
fterrani:patch-1
Aug 29, 2024
Merged

Fixed incorrect result type of duration-duration division#229
Jak-MS merged 1 commit intoMicrosoftDocs:mainfrom
fterrani:patch-1

Conversation

@fterrani
Copy link
Copy Markdown
Contributor

Hello.

I think the Division operator table has an incorrect value for the duration-duration division.

Table states duration / duration will produce a duration. However it seems to be contradicted by the subsequent Quotient of durations section:

The quotient of two durations is the number representing the quotient of the number of 100nanosecond ticks represented by the durations. For example:

#duration(2,0,0,0) / #duration(0,1,30,0) 
// 32

It is also contradicted by this simple test expression I created, which produces FALSE:

(#duration(1,0,0,0) / #duration(0,5,0,0)) is duration

Am I correct? Or is there something I don't understand?

@prmerger-automator
Copy link
Copy Markdown
Contributor

@fterrani : Thanks for your contribution! The author(s) have been notified to review your proposed change.

@learn-build-service-prod
Copy link
Copy Markdown
Contributor

Learn Build status updates of commit df842f2:

✅ Validation status: passed

File Status Preview URL Details
query-languages/m/m-spec-operators.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

@bgribaudo
Copy link
Copy Markdown
Contributor

bgribaudo commented Aug 29, 2024

Good catch!

Hmmm....looks like the fact that a duration can be divided by a duration is also missing from the second table at https://learn.microsoft.com/en-us/powerquery-m/m-spec-values#duration.

@Court72
Copy link
Copy Markdown
Contributor

Court72 commented Aug 29, 2024

@DougKlopfenstein

Can you review the proposed changes?

Important: When the changes are ready for publication, adding a #sign-off comment is the best way to signal that the PR is ready for the review team to merge.

#label:"aq-pr-triaged"
@MicrosoftDocs/public-repo-pr-review-team

@prmerger-automator prmerger-automator Bot added the aq-pr-triaged tracking label for the PR review team label Aug 29, 2024
@fterrani
Copy link
Copy Markdown
Contributor Author

To me, Values and Operator behavior both show the same operator-related information, one regrouping information by value and the other by operator.

We're in an annoying situation here:

  • either we repeat the information which makes the spec easier to understand / handier but is error prone
  • or we rework content to have operator information at a single location, making understanding harder but reducing error risk.

I wish there was an easy way to avoid that kind of issues in the long run... But that content rework looks costly to me and would probably produce more errors than not doing it at all. For the moment, I cannot see any quick and easy way to reduce repeated information. Adding links between these two pages for each matching section/table row sounds even more daunting.

It will probably be simpler / safer to just wait for people to report other errors (or for a (very) rainy weekend and a motivated person to check everything :) ).

@bgribaudo I'm not used to the spec enough to find the proper wording for the second table's Meaning column... :( I think it's better for everyone if I let you guys do this.

@DougKlopfenstein
Copy link
Copy Markdown
Contributor

Hi @bgribaudo - could you create another pull request for updating the duration table? If so, we can merge that into the docs as well.

@DougKlopfenstein
Copy link
Copy Markdown
Contributor

#sign-off

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants