I would like to give this task a try.

Just curious, is this just a cleanup of legacy API or is there any other issue with UNO API (performance, buggy,...)?

(In reply to Chen Huang from comment #3)
> I would like to give this task a try.
> 
> Just curious, is this just a cleanup of legacy API or is there any other
> issue with UNO API (performance, buggy,...)?

UNO is not a legacy API. It is our public external API. However as an external API it is quite slow as it has to do many checks (invalid user input, ...) and needs to take care of features not used during import like undo/redo, ...
In contrast to that using the direct calc internal API (normally around ScDocument) is as fast as possible and makes both debugging as well as profiling a lot easier.

Markus, thanks for the clear explanation.

I am assigning this to myself and start to work on it.

I have a few questions after looking at the code for a bit.

From tablebuffer.cxx:
void Table::finalizeImport() {
  ...
  PropertySet aPropSet( ... );
  if (no header) {
    aPropSet.setProperty( PROP_ContainsHeader, false);
  }
  ...
}
Why do we ever set PROP_ContainsHeader in this method given that aPropSet is a local variable and not passed back to caller?


I saw we have a ScDocument class and a ScDBData class, what's the difference between them?

If I understand correctly we don't really need to populate all the members of above classes and only need to properly fill maDestRange and mnTokenIndex in Table class. Is it correct?

(In reply to Chen Huang from comment #6)
> I have a few questions after looking at the code for a bit.
> 
> From tablebuffer.cxx:
> void Table::finalizeImport() {
>   ...
>   PropertySet aPropSet( ... );
>   if (no header) {
>     aPropSet.setProperty( PROP_ContainsHeader, false);
>   }
>   ...
> }
> Why do we ever set PROP_ContainsHeader in this method given that aPropSet is
> a local variable and not passed back to caller?

aPropSet is a wrapper around xDatabaseRange with a nicer interface. You can look into the PropertySet class to see how it stores xDatabaseRange in a member variable and in the call to setProperty forwards the call to this member variable.

> 
> 
> I saw we have a ScDocument class and a ScDBData class, what's the difference
> between them?

ScDocument represents a calc document. ScDBData a database range.

> 
> If I understand correctly we don't really need to populate all the members
> of above classes and only need to properly fill maDestRange and mnTokenIndex
> in Table class. Is it correct?

No. We need to set all properties that are handled at the moment through UNO directly to ScDBData.

A polite ping, still working on this issue ?

Sorry for no updates...

I am still working on this and should have followup updates soon.

My thinking is to add a helper in WorkbookGlobals like this:
ScDBData* createDatabaseRange(OUString& orName, range and other args) {
  NamedDBs& dbs = getScDocument().GetDBCollection()->getNamedDBs();
  if (dbs.findByUpperName(orName) != nullptr) { error }
  else {
    ScDBData* range = create ScDBData with orName, range and other args;
    dbs.insert(range);
    update maDBRangeName;
  }
}
Is this a reasonable way to achieve what we want?


And I have a few more questions:
- ScDBData starts a listener upon inserted into NamedDBs. But at this point we have not validated the range of the ScDBData yet so we are possibly listening on a wrong area, is it ok?

- Does mnTokenIndex in Table map to nIndex in ScDBData? If so how come they have different types? Although I think in reality it might not matter since int16 would be large enough to hold the number of sheets...

- I am having some trouble to see how the current Reference<XDatabaseRange> based implementation eventually transforms the range into a ScDBData, can you point me to where this occurs?

This seems to touch a lot of stuffs here and there and I am having trouble to fully understand the impact of my change given that this is my first task and I am still trying to get myself more familiar with the code base.

To not block this from being solved by others, I am making this back to "OPEN" and will look for some other tasks that have more limited scope.

resetting assignee.