Replace dump functions with operator<< overloads

[kounelisagis]

[sc-32959]

In TileDB-Py schema.dump() is called without any argument, leading writes to C stdout which Jupyter does not capture:

TileDB/tiledb/sm/array_schema/array_schema.cc

Lines 693 to 694 in 76dbda4

	if (out == nullptr)
	out = stdout;

Implementing a function that captures the output of the dump() to a string and then prints it to sys.stdout will fix the problem. Of course, ArraySchema::dump(std::string*) calls the dump() functions of other classes, so those need to be implemented as well.

But we don't even want to write functions called dump internally; that's a C idiom; we will reserve that term for the C API only. Since we're dealing with text output, everything here can be done better by overloading operator<< for std::ostream.

---

TYPE: IMPROVEMENT
DESC: Output of schema.dump() in TileDB-Py is not captured by Jupyter. Replacing dump functions with operator<< overloads will give the ability to print the resulted string from Python.

[eric-hughes-tiledb]

All these functions that implement a "dump" should be implemented as nonmember functions. This is in part to assure correctness. There's nothing in such a function for a schema that should ever need to rely on private member variables or private member functions. Using nonmember functions assures that they can't be called.

For historical reasons, our serialization and deserialization functions, both for storage and network, are implemented as member functions, but it would be better if they were moved out themselves. That's out of scope of this PR, but all the new code going in can be done this way.

We don't even want to write functions called dump internally; that's a C idiom; we will reserve that term for the C API only. Since we're dealing with text output, everything here can be done better by overloading operator<< for std::ostream. We don't want functions that return strings.

Changes required:

• No new dump functions.
• New overloads of operator<<.
• Implement C API functions with stringstream and fstream internally.

For an quick reference for the write function signatures, see the section "Stream Insertion and Extraction" in https://stackoverflow.com/questions/4421706/what-are-the-basic-rules-and-idioms-for-operator-overloading.

[eric-hughes-tiledb]

The various T::dump(FILE *) functions should all be inlined into their call sites in the C API implementation functions. The class-specific parts are not in operator<< functions and all that's remaining in these is a bit of interface code. Also in all of these function fwrite is better than fprintf.

[eric-hughes-tiledb]

Even without dealing with the issues with Filter, there are a number of mostly-small changes to be made.

For class Filter, it looks like the following would work:

1. A single definition of operator<< for filters.
2. A pure virtual function Filter::output that (1) calls. This should be protected.
3. An override of Filter::output in each class derived from filter. Also protected.
4. A friend declaration for operator<< so that Filter::output is available.

[kounelisagis]

Even without dealing with the issues with Filter, there are a number of mostly-small changes to be made.

For class Filter, it looks like the following would work:

1. A single definition of operator<< for filters.
2. A pure virtual function Filter::output that (1) calls. This should be protected.
3. An override of Filter::output in each class derived from filter. Also protected.
4. A friend declaration for operator<< so that Filter::output is available.

@eric-hughes-tiledb, making it a friend causes the need to be in tiledb::sm, causing all other operator<< overloads to also be in the namespace.

[eric-hughes-tiledb]

making it a friend causes the need to be in tiledb::sm

Friend declarations can be for namespace-qualified identifiers. friend ::something for the global namespace, for instance.

[eric-hughes-tiledb]

I won't approve this until ArraySchema::dump is gone, along with all of its ilk. Now that we have proper stream output, there's nothing left of value in these functions. All of the

All the assert statements here have to go. If the need for [[maybe_unused]] wasn't clear enough, assert does not always check the return value of an I/O function. (It does so only in some compiles, but not all.) We always need to check the return value of I/O functions.

The signature of Filter::output has to change. And we don't need Filter::dump any longer after this change; it and all its overrides can just go.

There are a number of places that use \n explicitly. These should be replaced with std::endl.

[eric-hughes-tiledb]

I couldn't resolve all your suggestions

I have. The problem with the regression test seems to have been a preexisting defect; there was missing inclusion of the external dimension header.

I went ahead and fixed up all my last comments as well, since I already was building the branch locally.

LGTM.

The result is a solid improvement over the prior.

[teo-tsirpanis]

These APIs do not exist.

[teo-tsirpanis]

Thanks!