Issue 45843

Summary: GETPIVOTDATA function missing ...
Product: Calc Reporter: mmeeks <mmeeks>
Component: codeAssignee: oc
Status: CLOSED FIXED QA Contact: issues@sc <issues>
Severity: Trivial    
Priority: P3 CC: issues, jody, kyoshida, manens, pagalmes.lists, rail_ooo, sragavan
Version: 680m79   
Target Milestone: ---   
Hardware: All   
OS: All   
Issue Type: ENHANCEMENT Latest Confirmation in: ---
Developer Difficulty: ---
Issue Depends on: 62490    
Issue Blocks:    
Attachments:
Description Flags
getpivotdata sample
none
Proposed patch for getpivotdata
none
proposed patch
none
new version of getpivotdata against m156
none
another example
none
Memory leak
none
TestCaseSpecification
none
Testdocuments for Test Case Specification none

Description mmeeks 2005-03-23 11:13:01 UTC
Just got a bug report about this - it seems a shame to have such a nice pivot
table implementation, but then miss this [ presuambly not overly difficult to
implement ] function often used to extract data before charting ;-)

Sample sheet attached.
Comment 1 mmeeks 2005-03-23 11:13:33 UTC
Created attachment 24203 [details]
getpivotdata sample
Comment 2 frank 2005-03-23 11:41:07 UTC
one for the requirements team
Comment 3 sragavan 2005-03-29 06:18:24 UTC
fst: Im working on this. Any comments/ directions are appreciated :-)
Comment 4 sragavan 2005-04-12 09:32:11 UTC
Created attachment 24961 [details]
Proposed patch for getpivotdata
Comment 5 frank 2005-04-12 09:39:49 UTC
Hi Niklas,

please check the patch.

Frank
Comment 6 mmeeks 2005-04-12 09:43:06 UTC
Niklas - we're really just looking for some brief comments on how to re-factor
this to make it acceptable :-)
Personally - I'd like to see (at least) the body of ScGetPivotData, the curious
query setup, the short lifetime ScDPObject safely encapsulated inside the
DataPilot code itself instead of here. But of course - none of this may be
necessary.
Comment 7 niklas.nebel 2005-04-12 16:26:33 UTC
Using ScSheetSourceDesc's aQueryParam has some fundamental limitations:
- It doesn't work with grouped fields (they are not in the source data that is
filtered).
- It doesn't work if the "Displayed value" setting is something different from
"Normal" (percentages will always be 100%, as the temporary table contains no
other entries).
- And of course it doesn't work for tables not based on sheet data.

I think it should be possible to find the values using the results that are
already stored in the live table's ScDPOutput object (find the selected items
using the MemberResult sequences, then look at that position in the DataResult
sequence). I haven't tried it, and there might be other problems, but it looks
like a better approach to me.
Comment 8 jodygoldberg 2005-04-12 22:42:06 UTC
The xls import and export will break when loading biff8 files.
In 97/2k getpivotdata had 2 args
    http://support.microsoft.com/?kbid=211949
It was not until biff8x in XP/2003 that the signature changed.

MS did not appear to change the function ordinal
Comment 9 sragavan 2005-04-13 10:19:08 UTC
nn: Thanks for your comment :-). Ill get back with a better patch.
Also will contact you on any issue....
Comment 10 sragavan 2005-04-19 10:03:34 UTC
Created attachment 25205 [details]
proposed patch
Comment 11 sragavan 2005-04-19 10:24:12 UTC
nn: i have done the way you have asked. Not depending upon aQueryParam. But it
returns only the total. But we should be looking at format spec in
http://office.microsoft.com/en-us/assistance/HP052091071033.aspx like with
filter params. My commented code, in the patch uses query param to do a query,
but gets the member from memberresults, and data from dataresults. Also it now
works independent of display type. ( i have set the reference to none). But it
doesnt work on the database/external service mode. Need your suggestion to make
it better.
Comment 12 niklas.nebel 2005-05-04 17:31:16 UTC
What I meant was, in ScDPOutput::GetPivotData, look through the MemberResult
sequences to find where in the output table the requested result is, and then
return that one from the DataResult sequence, instead of always using the total
result.

Oh, and can you please use a tab width of 4 (or, even better, no tabs at all)?
That would make your changes much easier to read.
Comment 13 jodygoldberg 2006-02-23 15:32:50 UTC
Created attachment 34406 [details]
new version of getpivotdata against m156
Comment 14 jodygoldberg 2006-02-23 17:30:04 UTC
Beyond the specific implementation of GETPIVOTDATA testing the function
highlighted a few problems with the pivot data import from xls.  The first is
that the function depends on the pivot tables existing to produce results.  The
xls importer does a recalc on import _before_ loading the pivots.  I attempted a
few elegant solutions to this but decided to fall back on a brute force
ScDPObject::Output call after the pivot is created.  As mentioned in #62490 that
call forces the generation of in sheet controls.  It has the additional side
effect of regenerating the view of the data pilot in the sheet, which triggers a
recalc for getpivottable users.
Comment 15 niklas.nebel 2006-02-24 13:39:24 UTC
lcl_getPivotDataApplyFilter doesn't work in the patch because the "CONTINUE"
handling is disturbed by result flags set for other fields (see the "Foo" column
in the first attachment).

But more fundamentally, the summing of results (accum) will always fail if a
different summary function or "displayed value" is selected. Getpivotdata should
be limited to values that are actually part of the DataPilot table, not try to
reimplement subtotal calculation.

Regarding Excel import: We don't refresh DataPilot tables after importing
because that could overwrite cells (for example, the source data might have been
changed without refreshing the pivot table in Excel).
Comment 16 jodygoldberg 2006-02-27 15:43:05 UTC
re: CONTINUE handling.
blah my other tests were working smoothly I'll examine this now to find the
misunderstanding.

> different summary function or "displayed value" is selected
The XL variant does not support custom agregation routines.  There seems to have
been some attempt but it does not work as far as I can tell.  By definition it
returns the sum.  As an extension we could consider supporting using the field
aggregator, or allowing user specified aggregation in the target field
specifier, but that would be an extension.

> Getpivotdata should be limited to values that are actually part of the
> DataPilot table
This is an interesting point for debate.  XL apparently has this restriction.  A
filter that has no visible subtotal will return a REF, but that seems like a
pointless limitation.

> Re: Refreshing after XL import.
I realize it is a kludge.  We're being hit with several layers of problem, that
was a solution that will solve some of them at the expense of others.
1) Having GetPivotData depend on ScDPOutput means that it is pulling values from
the _generated_ result.  On xls import we do not appear to be generating based
on the cached data and instead use the in sheet.  Throw in a few other bugs and
the DPOutput has a tenuous link to what is on the screen until we ::Output.  The
re are two potential solutions for GETPIVOTDATA.  Either pull the data off the
sheet, which seems impossibly kludgy (there is no layout information), or ensure
that the DPOutput is generated using the cached values with a matching
aggregation approach to Ms XL.  The latter seems best, but will take some time.  

2) GetPivotData is being calculated before the pivot is loaded and generating a
#REF.  We need to trigger a recalc for things that use them.  I looked at two
mechanisms.
    a) Keep track of all formulas that use GETPIVOTDATA on xls import and recalc
them after loading the pivots.  This became unyeildy as all of the potential
callers were considered (cells, validation, names).
    b) mark the content of the pivot table as being dirty to force all things
that reference it to recalc.  This is less clean, but much simpler.  It is also
a useful side effect of regeneration.

3) It was the easiest way to do what RefreshAfterLoad + SetButtons does for the
ods importer. Although it does lose us the autoformating.

Solving these problems will take some time.  For now, I'm advocating the 1 line
sledge hammer of ScDPObject::Output.
Comment 17 niklas.nebel 2006-02-27 17:14:32 UTC
Created attachment 34486 [details]
another example
Comment 18 niklas.nebel 2006-02-27 17:15:23 UTC
See the attached xls file for examples of different summary function or
displayed value, and how getpivotdata in Excel handles them.

The handling of Excel pivot tables after import is a separate issue, so we can
postpone that, but shouldn't assume that the tables are refreshed.
Comment 19 jodygoldberg 2006-02-27 17:32:22 UTC
Ahh, interesting.  I was testing with different aggregators on the col/row
fields  which produced some nonsensical results when using the autogenerated
getpivotdata feature.
Comment 20 niklas.nebel 2006-09-01 17:09:36 UTC
There's been no activity for 6 months now. Jody, are you going to continue with
this, or should someone else take over?
Comment 21 jodygoldberg 2006-09-01 19:02:59 UTC
The plan is to do some serious reworking of the datapilot internals to convert
the model from a string based to index based operation (something closer to the
XL 'pivotcache').  Given that the approach in the latest patch takes an invalid
approach I'll likely delay working on this until after the cache work is complete.

If someone else is interested they are welcome to it.
Comment 22 niklas.nebel 2007-02-09 10:54:32 UTC
I'm doing this myself now, using the MemberResult/DataResult approach.
Comment 23 kpalagin 2007-03-31 14:03:39 UTC
*** Issue 75921 has been marked as a duplicate of this issue. ***
Comment 24 niklas.nebel 2007-05-11 16:39:27 UTC
The implementation is in the CWS "getpivotdata".
Comment 25 niklas.nebel 2007-05-11 16:41:37 UTC
Reassigning to QA for verification.
Comment 26 rail_ooo 2007-06-05 22:40:03 UTC
The following XLS file causes to memory leak if you use this patch. Please take
a look.
Comment 27 rail_ooo 2007-06-05 22:42:06 UTC
Created attachment 45677 [details]
Memory leak
Comment 28 mmeeks 2007-06-06 09:22:29 UTC
rail - nn's solution is different; are you testing this cws ? or a custom build
with this patch :-)
Comment 29 rail_ooo 2007-06-06 10:38:56 UTC
I tested with the attached patch. Let me try getpivotdata CWS in this case.
Comment 30 oc 2007-06-22 12:42:05 UTC
Created attachment 46177 [details]
TestCaseSpecification
Comment 31 oc 2007-06-22 12:51:21 UTC
Created attachment 46178 [details]
Testdocuments for Test Case Specification
Comment 32 oc 2007-06-22 12:53:17 UTC
verified in internal build cws_getpivotdata
Comment 33 oc 2007-08-01 07:38:40 UTC
closed because fix available in SRC680_m221
Comment 34 niklas.nebel 2009-05-20 10:20:04 UTC
*** Issue 32305 has been marked as a duplicate of this issue. ***